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.

diff --git a/changes/cid_428 b/changes/cid_428
new file mode 100644 (file)
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 4869a2439ac3739dc860a7a4790ed0e4cd154764..2897fe10a12db2d0f05995f22fecaba0a0628b22 100644 (file)
--- 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);