CVE-2019-15026: Out-of-bounds Read (CWE-125)
value: memcached 1.5.16, when UNIX sockets are used, has a stack-based buffer over-read in conn_to_str in memcached.c.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.5
Published: 2019-08-30T15:15Z
Repository: https://github.com/memcached/memcached
Language: C
Files changed: memcached.c
Commit date: 2019-08-29 14:10:44 -0700
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.

Diff: diff --git a/memcached.c b/memcached.c
index 22915633..788647fe 100644
--- a/memcached.c
+++ b/memcached.c
@@ -3461,10 +3461,11 @@ static inline void get_conn_text(const conn *c, const int af,
                 char* addr, struct sockaddr *sock_addr) {
     char addr_text[MAXPATHLEN];
     addr_text[0] = '\0';
     const char *protoname = "?";
     unsigned short port = 0;
+    size_t pathlen = 0;
 
     switch (af) {
         case AF_INET:
             (void) inet_ntop(af,
                     &((struct sockaddr_in *)sock_addr)->sin_addr,
@@ -3486,14 +3487,31 @@ static inline void get_conn_text(const conn *c, const int af,
             port = ntohs(((struct sockaddr_in6 *)sock_addr)->sin6_port);
             protoname = IS_UDP(c->transport) ? "udp6" : "tcp6";
             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;
     }
 
     if (strlen(addr_text) < 2) {

Vulnerable code:                     sizeof(addr_text) - 1);
            addr_text[sizeof(addr_text)-1] = '\0';
Fixed code:     size_t pathlen = 0;
            // 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;
            }
                    pathlen);
            addr_text[pathlen] = '\0';