diff options
author | Oran Agra <oran@redislabs.com> | 2023-01-12 17:36:06 +0200 |
---|---|---|
committer | Oran Agra <oran@redislabs.com> | 2023-01-17 14:59:41 +0200 |
commit | c6bbfec2fffbd43ce53b89aac7983d80b3ca0bcc (patch) | |
tree | 589aa569c274f7a004b148a769e11419f9677d6c | |
parent | 4779ed5ebf22c13a254817bff74b686d5c8a6fee (diff) | |
download | redis-c6bbfec2fffbd43ce53b89aac7983d80b3ca0bcc.tar.gz |
Avoid integer overflows in SETRANGE and SORT (CVE-2022-35977)
Authenticated users issuing specially crafted SETRANGE and SORT(_RO)
commands can trigger an integer overflow, resulting with Redis attempting
to allocate impossible amounts of memory and abort with an OOM panic.
-rw-r--r-- | src/server.h | 6 | ||||
-rw-r--r-- | src/sort.c | 6 | ||||
-rw-r--r-- | src/t_string.c | 17 | ||||
-rw-r--r-- | tests/support/test.tcl | 6 | ||||
-rw-r--r-- | tests/unit/sort.tcl | 11 | ||||
-rw-r--r-- | tests/unit/type/string.tcl | 10 |
6 files changed, 48 insertions, 8 deletions
diff --git a/src/server.h b/src/server.h index 38774bbc2..200f60bc0 100644 --- a/src/server.h +++ b/src/server.h @@ -83,6 +83,12 @@ typedef long long ustime_t; /* microsecond time type. */ #include "endianconv.h" #include "crc64.h" +/* min/max */ +#undef min +#undef max +#define min(a, b) ((a) < (b) ? (a) : (b)) +#define max(a, b) ((a) > (b) ? (a) : (b)) + /* Error codes */ #define C_OK 0 #define C_ERR -1 diff --git a/src/sort.c b/src/sort.c index 7fc5c634e..6e015fc96 100644 --- a/src/sort.c +++ b/src/sort.c @@ -320,8 +320,10 @@ void sortCommand(client *c) { default: vectorlen = 0; serverPanic("Bad SORT type"); /* Avoid GCC warning */ } - /* Perform LIMIT start,count sanity checking. */ - start = (limit_start < 0) ? 0 : limit_start; + /* Perform LIMIT start,count sanity checking. + * And avoid integer overflow by limiting inputs to object sizes. */ + start = min(max(limit_start, 0), vectorlen); + limit_count = min(max(limit_count, -1), vectorlen); end = (limit_count < 0) ? vectorlen-1 : start+limit_count-1; if (start >= vectorlen) { start = vectorlen-1; diff --git a/src/t_string.c b/src/t_string.c index 34a452f49..9d2d17d61 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -34,8 +34,14 @@ * String Commands *----------------------------------------------------------------------------*/ -static int checkStringLength(client *c, long long size) { - if (!(c->flags & CLIENT_MASTER) && size > server.proto_max_bulk_len) { +static int checkStringLength(client *c, long long size, long long append) { + if (c->flags & CLIENT_MASTER) + return C_OK; + /* 'uint64_t' cast is there just to prevent undefined behavior on overflow */ + long long total = (uint64_t)size + append; + /* Test configured max-bulk-len represending a limit of the biggest string object, + * and also test for overflow. */ + if (total > server.proto_max_bulk_len || total < size || total < append) { addReplyError(c,"string exceeds maximum allowed size (proto-max-bulk-len)"); return C_ERR; } @@ -210,7 +216,7 @@ void setrangeCommand(client *c) { } /* Return when the resulting string exceeds allowed size */ - if (checkStringLength(c,offset+sdslen(value)) != C_OK) + if (checkStringLength(c,offset,sdslen(value)) != C_OK) return; o = createObject(OBJ_STRING,sdsnewlen(NULL, offset+sdslen(value))); @@ -230,7 +236,7 @@ void setrangeCommand(client *c) { } /* Return when the resulting string exceeds allowed size */ - if (checkStringLength(c,offset+sdslen(value)) != C_OK) + if (checkStringLength(c,offset,sdslen(value)) != C_OK) return; /* Create a copy when the object is shared or encoded. */ @@ -458,8 +464,7 @@ void appendCommand(client *c) { /* "append" is an argument, so always an sds */ append = c->argv[2]; - totlen = stringObjectLen(o)+sdslen(append->ptr); - if (checkStringLength(c,totlen) != C_OK) + if (checkStringLength(c,stringObjectLen(o),sdslen(append->ptr)) != C_OK) return; /* Append the value */ diff --git a/tests/support/test.tcl b/tests/support/test.tcl index 23015b3a7..ae0de4545 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -31,6 +31,12 @@ proc assert_match {pattern value} { } } +proc assert_not_equal {value expected {detail ""}} { + if {!($expected ne $value)} { + assert_failed "Expected '$value' not equal to '$expected'" $detail + } +} + proc assert_equal {value expected {detail ""}} { if {$expected ne $value} { if {$detail ne ""} { diff --git a/tests/unit/sort.tcl b/tests/unit/sort.tcl index 083c4540d..2f4c4db38 100644 --- a/tests/unit/sort.tcl +++ b/tests/unit/sort.tcl @@ -315,4 +315,15 @@ start_server { } } } + + test {SETRANGE with huge offset} { + r lpush L 2 1 0 + # expecting a different outcome on 32 and 64 bit systems + foreach value {9223372036854775807 2147483647} { + catch {[r sort_ro L by a limit 2 $value]} res + if {![string match "2" $res] && ![string match "*out of range*" $res]} { + assert_not_equal $res "expecting an error or 2" + } + } + } } diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 8126cdee8..b70a3317e 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -449,4 +449,14 @@ start_server {tags {"string"}} { test {LCS indexes with match len and minimum match len} { dict get [r STRALGO LCS IDX KEYS virus1 virus2 WITHMATCHLEN MINMATCHLEN 5] matches } {{{1 222} {13 234} 222}} + + test {SETRANGE with huge offset} { + foreach value {9223372036854775807 2147483647} { + catch {[r setrange K $value A]} res + # expecting a different error on 32 and 64 bit systems + if {![string match "*string exceeds maximum allowed size*" $res] && ![string match "*out of range*" $res]} { + assert_equal $res "expecting an error" + } + } + } } |