diff --git a/memcached.c b/memcached.c index a950fa4..63848e2 100644 --- a/memcached.c +++ b/memcached.c @@ -3395,6 +3395,7 @@ static void conn_to_str(const conn *c, char *buf) { struct sockaddr *addr = (void *)&c->request_addr; int af; unsigned short port = 0; + size_t pathlen = 0; /* For listen ports and idle UDP ports, show listen address */ if (c->state == conn_listening || @@ -3436,10 +3437,27 @@ static void conn_to_str(const conn *c, char *buf) { 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 *)addr)->sun_path); + if (MAXPATHLEN <= pathlen) { + pathlen = MAXPATHLEN - 1; + } strncpy(addr_text, ((struct sockaddr_un *)addr)->sun_path, - sizeof(addr_text) - 1); - addr_text[sizeof(addr_text)-1] = '\0'; + pathlen); + addr_text[pathlen] = '\0'; protoname = "unix"; break; }