Make sure Unix socket path is null-terminated, per unix(7)

ncat/ncat_connect.c

@@ -1008,8 +1008,7 @@ int ncat_connect(void)
                     bye("Failed to create name for temporary DGRAM source Unix domain socket (tempnam).");
 #endif
 
-                srcaddr.un.sun_family = AF_UNIX;
+                NCAT_INIT_SUN(&srcaddr, tmp_name);
-                strncpy(srcaddr.un.sun_path, tmp_name, sizeof(srcaddr.un.sun_path));
                 free (tmp_name);
             }
 

ncat/ncat_listen.c

@@ -406,24 +406,21 @@ int ncat_listen()
    watch set. */
 static void handle_connection(int socket_accept, int type, fd_set *listen_fds)
 {
-    union sockaddr_u remoteaddr;
-    socklen_t ss_len;
     struct fdinfo s = { 0 };
     int conn_count;
 
     zmem(&s, sizeof(s));
-    zmem(&remoteaddr, sizeof(remoteaddr.storage));
 
-    ss_len = sizeof(remoteaddr.storage);
+    s.ss_len = sizeof(s.remoteaddr.storage);
 
     errno = 0;
     if (type == SOCK_STREAM) {
-      s.fd = accept(socket_accept, &remoteaddr.sockaddr, &ss_len);
+      s.fd = accept(socket_accept, &s.remoteaddr.sockaddr, &s.ss_len);
     }
     else {
       char buf[4] = {0};
       int nbytes = recvfrom(socket_accept, buf, sizeof(buf), MSG_PEEK,
-          &remoteaddr.sockaddr, &ss_len);
+          &s.remoteaddr.sockaddr, &s.ss_len);
       if (nbytes < 0) {
         loguser("%s.\n", socket_strerror(socket_errno()));
         return;
@@ -432,23 +429,33 @@ static void handle_connection(int socket_accept, int type, fd_set *listen_fds)
        * We're using connected udp. This has the down side of only
        * being able to handle one udp client at a time
        */
-      Connect(socket_accept, &remoteaddr.sockaddr, ss_len);
+      Connect(socket_accept, &s.remoteaddr.sockaddr, s.ss_len);
       s.fd = socket_accept;
-      // Remove this socket from listening and put a new one up.
+      /* If we expect new connections, we'll have to open a new listening
-      for (int i = 0; i < num_listenaddrs; i++) {
+       * socket to replace the one we just connected to a single client. */
-        if (listen_socket[i] == socket_accept) {
+      if ((o.keepopen || o.broker)
-          struct fdinfo *lfdi = get_fdinfo(&client_fdlist, socket_accept);
+#if HAVE_SYS_UN_H
-          union sockaddr_u localaddr = lfdi->remoteaddr;
+          /* unless it's a UNIX socket, since we get EADDRINUSE when we try to bind */
-          checked_fd_clr(socket_accept, &master_readfds);
+          && s.remoteaddr.storage.ss_family != AF_UNIX
-          checked_fd_clr(socket_accept, listen_fds);
+#endif
-          rm_fd(&client_fdlist, socket_accept);
+        ) {
-          listen_socket[i] = new_listen_socket(type, (o.af == AF_INET || o.af == AF_INET6) ? o.proto : 0, &localaddr, listen_fds);
+        for (int i = 0; i < num_listenaddrs; i++) {
-          if (listen_socket[i] < 0) {
+          if (listen_socket[i] == socket_accept) {
-            bye("do_listen(\"%s\"): %s\n", socktop(&listenaddrs[i], 0), socket_strerror(socket_errno()));
+            struct fdinfo *lfdi = get_fdinfo(&client_fdlist, socket_accept);
-            return;
+            union sockaddr_u localaddr = lfdi->remoteaddr;
+            listen_socket[i] = new_listen_socket(type, (o.af == AF_INET || o.af == AF_INET6) ? o.proto : 0, &localaddr, listen_fds);
+            if (listen_socket[i] < 0) {
+              bye("do_listen(\"%s\"): %s\n", socktop(&listenaddrs[i], 0), socket_strerror(socket_errno()));
+              return;
+            }
+            break;
           }
         }
       }
+      /* Remove this socket from listening */
+      checked_fd_clr(socket_accept, &master_readfds);
+      checked_fd_clr(socket_accept, listen_fds);
+      rm_fd(&client_fdlist, socket_accept);
     }
 
     if (s.fd < 0) {
@@ -462,14 +469,18 @@ static void handle_connection(int socket_accept, int type, fd_set *listen_fds)
     if (!o.keepopen && !o.broker) {
         int i;
         for (i = 0; i < num_listenaddrs; i++) {
-            Close(listen_socket[i]);
+            /* If */
-            checked_fd_clr(listen_socket[i], &master_readfds);
+            if (listen_socket[i] >= 0 && checked_fd_isset(listen_socket[i], listen_fds)) {
-            rm_fd(&client_fdlist, listen_socket[i]);
+              Close(listen_socket[i]);
+              checked_fd_clr(listen_socket[i], &master_readfds);
+              rm_fd(&client_fdlist, listen_socket[i]);
+              listen_socket[i] = -1;
+            }
         }
     }
 
     if (o.verbose) {
-        loguser("Connection from %s", socktop(&remoteaddr, ss_len));
+        loguser("Connection from %s", socktop(&s.remoteaddr, s.ss_len));
         if (o.chat)
             loguser_noprefix(" on file descriptor %d", s.fd);
         loguser_noprefix(".\n");
@@ -483,15 +494,13 @@ static void handle_connection(int socket_accept, int type, fd_set *listen_fds)
         Close(s.fd);
         return;
     }
-    if (!allow_access(&remoteaddr)) {
+    if (!allow_access(&s.remoteaddr)) {
         if (o.verbose)
             loguser("New connection denied: not allowed\n");
         Close(s.fd);
         return;
     }
 
-    s.remoteaddr = remoteaddr;
-
     conn_inc++;
 
     unblock_socket(s.fd);
@@ -770,14 +779,14 @@ static void shutdown_sockets(int how)
 }
 
 /* Announce the new connection and who is already connected. */
-static int chat_announce_connect(int fd, const union sockaddr_u *su)
+static int chat_announce_connect(const struct fdinfo *fdi)
 {
     char *buf = NULL;
     size_t size = 0, offset = 0;
     int i, count, ret;
 
     strbuf_sprintf(&buf, &size, &offset,
-        "<announce> %s is connected as <user%d>.\n", socktop(su, 0), fd);
+        "<announce> %s is connected as <user%d>.\n", socktop(&fdi->remoteaddr, fdi->ss_len), fdi->fd);
 
     strbuf_sprintf(&buf, &size, &offset, "<announce> already connected: ");
     count = 0;
@@ -785,7 +794,7 @@ static int chat_announce_connect(int fd, const union sockaddr_u *su)
         union sockaddr_u tsu;
         socklen_t len = sizeof(tsu.storage);
 
-        if (i == fd || !checked_fd_isset(i, &master_broadcastfds))
+        if (i == fdi->fd || !checked_fd_isset(i, &master_broadcastfds))
             continue;
 
         if (getpeername(i, &tsu.sockaddr, &len) == -1)

ncat/ncat_main.c

@@ -794,8 +794,7 @@ int main(int argc, char *argv[])
          * If it's not valid, it will fail later! */
         if (o.af == AF_UNIX) {
             if (o.proto == IPPROTO_UDP) {
-                srcaddr.un.sun_family = AF_UNIX;
+                NCAT_INIT_SUN(&srcaddr, source);
-                strncpy(srcaddr.un.sun_path, source, sizeof(srcaddr.un.sun_path));
                 srcaddrlen = SUN_LEN(&srcaddr.un);
             }
             else
@@ -853,9 +852,7 @@ int main(int argc, char *argv[])
       case 1:
 #if HAVE_SYS_UN_H
         if (o.af == AF_UNIX) {
-            memset(&targetaddrs->addr.storage, 0, sizeof(struct sockaddr_un));
+            NCAT_INIT_SUN(&targetaddrs->addr, argv[optind]);
-            targetaddrs->addr.un.sun_family = AF_UNIX;
-            strncpy(targetaddrs->addr.un.sun_path, argv[optind], sizeof(targetaddrs->addr.un.sun_path));
             targetaddrs->addrlen = SUN_LEN(&targetaddrs->addr.un);
             o.sslservername = o.target = argv[optind];
             break;

ncat/sockaddr_u.h

@@ -65,6 +65,18 @@
 #ifndef SOCKADDR_U_H_
 #define SOCKADDR_U_H_
 
+#ifdef WIN32
+# include <ws2def.h>
+#endif
+#if HAVE_NETINET_IN_H
+# include <netinet/in.h>
+#endif
+#if HAVE_SYS_SOCKET_H
+# include <sys/socket.h>
+#endif
+#if HAVE_SYS_UN_H
+# include <sys/un.h>
+#endif
 #if HAVE_LINUX_VM_SOCKETS_H
 #include <linux/vm_sockets.h>
 #endif
@@ -82,4 +94,37 @@ union sockaddr_u {
     struct sockaddr sockaddr;
 };
 
+static inline socklen_t get_socklen(const union sockaddr_u *s)
+{
+    switch(s->storage.ss_family) {
+#ifdef HAVE_SYS_UN_H
+      case AF_UNIX:
+        return SUN_LEN(&s->un);
+        break;
+#endif
+#ifdef HAVE_LINUX_VM_SOCKETS_H
+      case AF_VSOCK:
+        return sizeof(struct sockaddr_vm);
+        break;
+#endif
+#ifdef HAVE_SOCKADDR_SA_LEN
+      default:
+        return s->sockaddr.sa_len;
+        break;
+#else
+      case AF_INET:
+        return sizeof(struct sockaddr_in);
+        break;
+#ifdef AF_INET6
+      case AF_INET6:
+        return sizeof(struct sockaddr_in6);
+        break;
+#endif
+      default:
+        return sizeof(union sockaddr_u);
+        break;
+#endif
+    }
+    return 0;
+}
 #endif

ncat/util.h

@@ -70,8 +70,20 @@
 #include <netinet/in.h>
 #endif
 
+#include "sockaddr_u.h"
+
 #if HAVE_SYS_UN_H
 #include <sys/un.h>
+#include <string.h>
+
+#define NCAT_INIT_SUN(_Sock, _Source) do { \
+  memset(_Sock, 0, sizeof(union sockaddr_u)); \
+  (_Sock)->un.sun_family = AF_UNIX; \
+  if (strlen(_Source) > sizeof((_Sock)->un.sun_path) - 1) \
+    bye("Socket path length is too long. Max: %lu", sizeof((_Sock)->un.sun_path) - 1); \
+  strncpy((_Sock)->un.sun_path, _Source, sizeof((_Sock)->un.sun_path) - 1); \
+} while (0);
+
 #endif
 
 #ifdef HAVE_OPENSSL
@@ -86,8 +98,6 @@ size_t smul(size_t, size_t);
 void windows_init();
 #endif
 
-#include "sockaddr_u.h"
-
 void loguser(const char *fmt, ...)
      __attribute__ ((format (printf, 1, 2)));
 void loguser_noprefix(const char *fmt, ...)
@@ -143,6 +153,7 @@ void ms_to_timeval(struct timeval *tv, long ms)
 struct fdinfo {
     int fd;
     union sockaddr_u remoteaddr;
+    socklen_t ss_len;
 #ifdef HAVE_OPENSSL
     SSL *ssl;
 #endif