59 lines
2.6 KiB
Diff
59 lines
2.6 KiB
Diff
|
From 554b56687a19300a75ec24184746b5512580c819 Mon Sep 17 00:00:00 2001
|
||
|
|
Date: Fri, 6 Sep 2019 15:47:09 +0100
|
||
|
|
Subject: [PATCH] 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.
|
||
|
|
---
|
||
|
|
memcached.c | 22 ++++++++++++++++++++--
|
||
|
|
1 file changed, 20 insertions(+), 2 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/memcached.c b/memcached.c
|
||
|
|
index 5e170bf..eee69e9 100644
|
||
|
|
--- a/memcached.c
|
||
|
|
+++ b/memcached.c
|
||
|
|
@@ -2683,6 +2683,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 ||
|
||
|
|
@@ -2724,10 +2725,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;
|
||
|
|
}
|