From e5ff3ea8af63bcfea135dd488eb76788a757bdc1 Mon Sep 17 00:00:00 2001 From: Kevin Binetruy Date: Tue, 13 Oct 2026 14:55:05 +0000 Subject: [PATCH] MT#55283 redis: do not read past a short INFO reply Ref #2211 redis_connect() finds "role:" in the INFO reply and compares what follows with memcmp(): 11 bytes for "role:master", 19 for "role:active-replica" and 10 for "role:slave". memcmp() may read all n bytes, so a reply that ends less than 19 bytes after "role:" (for example a replica's "role:slave\r\n" with nothing after it) is read past its end. Real Redis and KeyDB servers send many more lines after the role, so this does not show in practice, but AddressSanitizer aborts on it (heap-buffer-overflow in the memcmp interceptor) with a minimal reply, such as one from a test server or a proxy. Use strncmp(): hiredis NUL-terminates the reply string, so the compare stops at its end. Tested by auto-daemon-tests-redis-errors.pl, added later in this series, which answers a reconnect's INFO with just "role:slave": an ASan build aborts there without this change. The change is early in the series so that it can be taken on its own. Change-Id: Icdf3d3a23464d861f57406436328d0b166794362 --- daemon/redis.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/redis.c b/daemon/redis.c index e8e9d7a69..d25a1dcd7 100644 --- a/daemon/redis.c +++ b/daemon/redis.c @@ -346,7 +346,7 @@ static int redis_connect(struct redis *r, int wait, bool resolve) { goto err3; } - if (!memcmp(s, "role:master", 11) || !memcmp(s, "role:active-replica", 19)) { + if (!strncmp(s, "role:master", 11) || !strncmp(s, "role:active-replica", 19)) { if (r->role == MASTER_REDIS_ROLE || r->role == ANY_REDIS_ROLE) { ilog(LOG_INFO, "Connected to Redis %s in master mode", endpoint_print_buf(&r->endpoint)); @@ -356,7 +356,7 @@ static int redis_connect(struct redis *r, int wait, bool resolve) { endpoint_print_buf(&r->endpoint)); goto next; } - } else if (!memcmp(s, "role:slave", 10)) { + } else if (!strncmp(s, "role:slave", 10)) { if (r->role == SLAVE_REDIS_ROLE || r->role == ANY_REDIS_ROLE) { ilog(LOG_INFO, "Connected to Redis %s in slave mode", endpoint_print_buf(&r->endpoint));