commit 959da6b7f2b5ed63426fd12a9046ac06033f6db1 Author: Nick Mathewson nickm@torproject.org Date: Fri Jul 1 12:06:54 2011 -0400
Use strlcpy in create_unix_sockaddr()
Using strncpy meant that if listenaddress were ever >= sizeof(sockaddr_un.sun_path), we would fail to nul-terminate sun_path. This isn't a big deal: we never read sun_path, and the kernel is smart enough to reject the sockaddr_un if it isn't nul-terminated. Nonetheless, it's a dumb failure mode. Instead, we should reject addresses that don't fit in sockaddr_un.sun_path.
Coverity found this; it's CID 428. Bugfix on 0.2.0.3-alpha. --- changes/cid_428 | 5 +++++ src/or/connection.c | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/changes/cid_428 b/changes/cid_428 new file mode 100644 index 0000000..cb0fc8c --- /dev/null +++ b/changes/cid_428 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Always NUL-terminate the sun_path field of a sockaddr_un before + passing it to the kernel. (Not a security issue: kernels are + smart enough to reject bad sockaddr_uns.) Found by Coverity; CID + # 428. Bugfix on Tor 0.2.0.3-alpha. diff --git a/src/or/connection.c b/src/or/connection.c index 4869a24..2897fe1 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -804,7 +804,13 @@ create_unix_sockaddr(const char *listenaddress, char **readable_address,
sockaddr = tor_malloc_zero(sizeof(struct sockaddr_un)); sockaddr->sun_family = AF_UNIX; - strncpy(sockaddr->sun_path, listenaddress, sizeof(sockaddr->sun_path)); + if (strlcpy(sockaddr->sun_path, listenaddress, sizeof(sockaddr->sun_path)) + >= sizeof(sockaddr->sun_path)) { + log_warn(LD_CONFIG, "Unix socket path '%s' is too long to fit.", + escaped(listenaddress)); + tor_free(sockaddr); + return NULL; + }
if (readable_address) *readable_address = tor_strdup(listenaddress);