From 6d21560190fd5b09ff849ad1777e868d5e78da5f Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 22 Sep 2022 11:55:53 +0300 Subject: [PATCH] Fix heap overflow vulnerability in XAUTOCLAIM (CVE-2022-35951) (#11301) Executing an XAUTOCLAIM command on a stream key in a specific state, with a specially crafted COUNT argument may cause an integer overflow, a subsequent heap overflow, and potentially lead to remote code execution. The problem affects Redis versions 7.0.0 or newer. --- src/t_stream.c | 13 ++++++++++--- tests/unit/type/stream-cgroups.tcl | 4 ++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/t_stream.c b/src/t_stream.c index 2bcae25b4..ea38ab45d 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -3334,6 +3334,7 @@ void xautoclaimCommand(client *c) { robj *o = lookupKeyRead(c->db,c->argv[1]); long long minidle; /* Minimum idle time argument, in milliseconds. */ long count = 100; /* Maximum entries to claim. */ + const unsigned attempts_factor = 10; streamID startid; int startex; int justid = 0; @@ -3356,7 +3357,8 @@ void xautoclaimCommand(client *c) { int moreargs = (c->argc-1) - j; /* Number of additional arguments. */ char *opt = c->argv[j]->ptr; if (!strcasecmp(opt,"COUNT") && moreargs) { - if (getRangeLongFromObjectOrReply(c,c->argv[j+1],1,LONG_MAX,&count,"COUNT must be > 0") != C_OK) + long max_count = LONG_MAX / (max(sizeof(streamID), attempts_factor)); + if (getRangeLongFromObjectOrReply(c,c->argv[j+1],1,max_count,&count,"COUNT must be > 0") != C_OK) return; j++; } else if (!strcasecmp(opt,"JUSTID")) { @@ -3383,9 +3385,15 @@ void xautoclaimCommand(client *c) { return; } + streamID *deleted_ids = ztrymalloc(count * sizeof(streamID)); + if (!deleted_ids) { + addReplyError(c, "Insufficient memory, failed allocating transient memory, COUNT too high."); + return; + } + /* Do the actual claiming. */ streamConsumer *consumer = NULL; - long long attempts = count*10; + long long attempts = count * attempts_factor; addReplyArrayLen(c, 3); /* We add another reply later */ void *endidptr = addReplyDeferredLen(c); /* reply[0] */ @@ -3399,7 +3407,6 @@ void xautoclaimCommand(client *c) { size_t arraylen = 0; mstime_t now = mstime(); sds name = c->argv[3]->ptr; - streamID *deleted_ids = zmalloc(count * sizeof(streamID)); int deleted_id_num = 0; while (attempts-- && count && raxNext(&ri)) { streamNACK *nack = ri.data; diff --git a/tests/unit/type/stream-cgroups.tcl b/tests/unit/type/stream-cgroups.tcl index 398ebf2e1..928a83b07 100644 --- a/tests/unit/type/stream-cgroups.tcl +++ b/tests/unit/type/stream-cgroups.tcl @@ -741,6 +741,10 @@ start_server { assert_equal [r XPENDING x grp - + 10 Alice] {} } + test {XAUTOCLAIM with out of range count} { + assert_error {ERR COUNT*} {r XAUTOCLAIM x grp Bob 0 3-0 COUNT 8070450532247928833} + } + test {XCLAIM with trimming} { r DEL x r config set stream-node-max-entries 2