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
pull/2217/head
Kevin Binetruy 1 month ago committed by Richard Fuchs
parent 20bcdce73c
commit e5ff3ea8af

@ -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));

Loading…
Cancel
Save