Vulnerability: memcached 1.5.16, when UNIX sockets are used, has a stack-based buffer over-read in conn_to_str in memcached.c.
Vulnerability ID: CVE-2019-15026
CWE: CWE-125
Severity score: 2.9
Project: https://github.com/memcached/memcached
Published: 2019-08-30
File: memcached.c
Commit message: fix strncpy call to avoid ASAN violation

Ensure we're only reading to the size of the smallest buffer, since
they're both on the stack and could potentially overlap. Overlapping is
defined as ... undefined behavior. I've looked through all available
implementations of strncpy and they still only copy from the first \0
found.

We'll also never read past the end of sun_path since we _supply_
sun_path with a proper null terminator.
Commit author: dormando

Vulnerable version (before fix):
 static inline void get_conn_text(const conn *c, const int af,     addr_text[0] = '\0';     const char *protoname = "?";     unsigned short port = 0;      switch (af) {         case AF_INET: static inline void get_conn_text(const conn *c, const int af,             break;          case AF_UNIX:             strncpy(addr_text,                     ((struct sockaddr_un *)sock_addr)->sun_path,                     sizeof(addr_text) - 1);             addr_text[sizeof(addr_text)-1] = '\0';             protoname = "unix";             break;     }

Fixed version (after fix):
 static inline void get_conn_text(const conn *c, const int af,     addr_text[0] = '\0';     const char *protoname = "?";     unsigned short port = 0;     size_t pathlen = 0;      switch (af) {         case AF_INET: static inline void get_conn_text(const conn *c, const int af,             break;          case AF_UNIX:             // this strncpy call originally could piss off an address             // sanitizer; we supplied the size of the dest buf as a limiter,             // but optimized versions of strncpy could read past the end of             // *src while looking for a null terminator. Since buf and             // sun_path here are both on the stack they could even overlap,             // which is "undefined". In all OSS versions of strncpy I could             // find this has no effect; it'll still only copy until the first null             // terminator is found. Thus it's possible to get the OS to             // examine past the end of sun_path but it's unclear to me if this             // can cause any actual problem.             //             // We need a safe_strncpy util function but I'll punt on figuring             // that out for now.             pathlen = sizeof(((struct sockaddr_un *)sock_addr)->sun_path);             if (MAXPATHLEN <= pathlen) {                 pathlen = MAXPATHLEN - 1;             }             strncpy(addr_text,                     ((struct sockaddr_un *)sock_addr)->sun_path,                     pathlen);             addr_text[pathlen] = '\0';             protoname = "unix";             break;     }

Patch:
patch: @@ -3463,6 +3463,7 @@ static inline void get_conn_text(const conn *c, const int af,
     addr_text[0] = '\0';
     const char *protoname = "?";
     unsigned short port = 0;
+    size_t pathlen = 0;
 
     switch (af) {
         case AF_INET:
@@ -3488,10 +3489,27 @@ static inline void get_conn_text(const conn *c, const int af,
             break;
 
         case AF_UNIX:
+            // this strncpy call originally could piss off an address
+            // sanitizer; we supplied the size of the dest buf as a limiter,
+            // but optimized versions of strncpy could read past the end of
+            // *src while looking for a null terminator. Since buf and
+            // sun_path here are both on the stack they could even overlap,
+            // which is "undefined". In all OSS versions of strncpy I could
+            // find this has no effect; it'll still only copy until the first null
+            // terminator is found. Thus it's possible to get the OS to
+            // examine past the end of sun_path but it's unclear to me if this
+            // can cause any actual problem.
+            //
+            // We need a safe_strncpy util function but I'll punt on figuring
+            // that out for now.
+            pathlen = sizeof(((struct sockaddr_un *)sock_addr)->sun_path);
+            if (MAXPATHLEN <= pathlen) {
+                pathlen = MAXPATHLEN - 1;
+            }
             strncpy(addr_text,
                     ((struct sockaddr_un *)sock_addr)->sun_path,
-                    sizeof(addr_text) - 1);
-            addr_text[sizeof(addr_text)-1] = '\0';
+                    pathlen);
+            addr_text[pathlen] = '\0';
             protoname = "unix";
             break;
     }
path: memcached.c