Skip to content

Commit 554b566

Browse files
committed
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.
1 parent 938154d commit 554b566

File tree

1 file changed

+20
-2
lines changed

1 file changed

+20
-2
lines changed

memcached.c

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3463,6 +3463,7 @@ static inline void get_conn_text(const conn *c, const int af,
34633463
addr_text[0] = '\0';
34643464
const char *protoname = "?";
34653465
unsigned short port = 0;
3466+
size_t pathlen = 0;
34663467

34673468
switch (af) {
34683469
case AF_INET:
@@ -3488,10 +3489,27 @@ static inline void get_conn_text(const conn *c, const int af,
34883489
break;
34893490

34903491
case AF_UNIX:
3492+
// this strncpy call originally could piss off an address
3493+
// sanitizer; we supplied the size of the dest buf as a limiter,
3494+
// but optimized versions of strncpy could read past the end of
3495+
// *src while looking for a null terminator. Since buf and
3496+
// sun_path here are both on the stack they could even overlap,
3497+
// which is "undefined". In all OSS versions of strncpy I could
3498+
// find this has no effect; it'll still only copy until the first null
3499+
// terminator is found. Thus it's possible to get the OS to
3500+
// examine past the end of sun_path but it's unclear to me if this
3501+
// can cause any actual problem.
3502+
//
3503+
// We need a safe_strncpy util function but I'll punt on figuring
3504+
// that out for now.
3505+
pathlen = sizeof(((struct sockaddr_un *)sock_addr)->sun_path);
3506+
if (MAXPATHLEN <= pathlen) {
3507+
pathlen = MAXPATHLEN - 1;
3508+
}
34913509
strncpy(addr_text,
34923510
((struct sockaddr_un *)sock_addr)->sun_path,
3493-
sizeof(addr_text) - 1);
3494-
addr_text[sizeof(addr_text)-1] = '\0';
3511+
pathlen);
3512+
addr_text[pathlen] = '\0';
34953513
protoname = "unix";
34963514
break;
34973515
}

0 commit comments

Comments
 (0)