Skip to content

Commit

Permalink
Fix redis-cli / redis-sential overflow on some platforms (CVE-2021-32…
Browse files Browse the repository at this point in the history
…762) (#9587)

The redis-cli command line tool and redis-sentinel service may be vulnerable
to integer overflow when parsing specially crafted large multi-bulk network
replies. This is a result of a vulnerability in the underlying hiredis
library which does not perform an overflow check before calling the calloc()
heap allocation function.

This issue only impacts systems with heap allocators that do not perform their
own overflow checks. Most modern systems do and are therefore not likely to
be affected. Furthermore, by default redis-sentinel uses the jemalloc allocator
which is also not vulnerable.

Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
  • Loading branch information
oranagra and yossigo committed Oct 4, 2021
1 parent 7cb89a5 commit 0215324
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
1 change: 1 addition & 0 deletions deps/hiredis/hiredis.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ static void *createArrayObject(const redisReadTask *task, size_t elements) {
return NULL;

if (elements > 0) {
if (SIZE_MAX / sizeof(redisReply*) < elements) return NULL; /* Don't overflow */

This comment has been minimized.

Copy link
@natoscott

natoscott Oct 10, 2021

Contributor

@yossigo there's a memory leak here now - we need to call freeReplyObject(r) before returning NULL .

This comment has been minimized.

Copy link
@yossigo

yossigo Oct 11, 2021

Author Member

Right, I see it's properly fixed in hiredis and we have diverged in other ways as well, so it's time to resync. Thanks!

r->element = hi_calloc(elements,sizeof(redisReply*));
if (r->element == NULL) {
freeReplyObject(r);
Expand Down
14 changes: 14 additions & 0 deletions deps/hiredis/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,20 @@ static void test_reply_reader(void) {
freeReplyObject(reply);
redisReaderFree(reader);

test("Multi-bulk never overflows regardless of maxelements: ");
size_t bad_mbulk_len = (SIZE_MAX / sizeof(void *)) + 3;
char bad_mbulk_reply[100];
snprintf(bad_mbulk_reply, sizeof(bad_mbulk_reply), "*%llu\r\n+asdf\r\n",
(unsigned long long) bad_mbulk_len);

reader = redisReaderCreate();
reader->maxelements = 0; /* Don't rely on default limit */
redisReaderFeed(reader, bad_mbulk_reply, strlen(bad_mbulk_reply));
ret = redisReaderGetReply(reader,&reply);
test_cond(ret == REDIS_ERR && strcasecmp(reader->errstr, "Out of memory") == 0);
freeReplyObject(reply);
redisReaderFree(reader);

#if LLONG_MAX > SIZE_MAX
test("Set error when array > SIZE_MAX: ");
reader = redisReaderCreate();
Expand Down

0 comments on commit 0215324

Please sign in to comment.