Browse Source

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.
Nick Mathewson 13 years ago
parent
commit
959da6b7f2
2 changed files with 12 additions and 1 deletions
  1. 5 0
      changes/cid_428
  2. 7 1
      src/or/connection.c

+ 5 - 0
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.

+ 7 - 1
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);