diff --git a/src/sort.cpp b/src/sort.cpp index e31e0039c..5a074d017 100644 --- a/src/sort.cpp +++ b/src/sort.cpp @@ -194,8 +194,8 @@ void sortCommand(client *c) { list *operations; unsigned int outputlen = 0; int desc = 0, alpha = 0; - long limit_start = 0, limit_count = -1, start, end; - int j, dontsort = 0, vectorlen; + long limit_start = 0, limit_count = -1, start, end, vectorlen; + int j, dontsort = 0; int getop = 0; /* GET operation counter */ int int_conversion_error = 0; int syntax_error = 0; @@ -321,8 +321,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 = std::min(std::max(limit_start, (long)0), vectorlen); + limit_count = std::min(std::max(limit_count, (long)-1), vectorlen); end = (limit_count < 0) ? vectorlen-1 : start+limit_count-1; if (start >= vectorlen) { start = vectorlen-1; diff --git a/src/t_hash.cpp b/src/t_hash.cpp index 9eddbb449..ed1bb4d63 100644 --- a/src/t_hash.cpp +++ b/src/t_hash.cpp @@ -1225,8 +1225,13 @@ void hrandfieldCommand(client *c) { if (c->argc > 4 || (c->argc == 4 && strcasecmp(szFromObj(c->argv[3]),"withvalues"))) { addReplyErrorObject(c,shared.syntaxerr); return; - } else if (c->argc == 4) + } else if (c->argc == 4) { withvalues = 1; + if (l < LONG_MIN/2 || l > LONG_MAX/2) { + addReplyError(c,"value is out of range"); + return; + } + } hrandfieldWithCountCommand(c, l, withvalues); return; } diff --git a/src/t_string.cpp b/src/t_string.cpp index b9a417061..f946ba3a3 100644 --- a/src/t_string.cpp +++ b/src/t_string.cpp @@ -38,8 +38,14 @@ int getGenericCommand(client *c); * String Commands *----------------------------------------------------------------------------*/ -static int checkStringLength(client *c, long long size) { - if (!(c->flags & CLIENT_MASTER) && size > g_pserver->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 > g_pserver->proto_max_bulk_len || total < size || total < append) { addReplyError(c,"string exceeds maximum allowed size (proto-max-bulk-len)"); return C_ERR; } @@ -445,7 +451,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))); @@ -465,7 +471,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. */ @@ -685,8 +691,7 @@ void appendCommand(client *c) { /* "append" is an argument, so always an sds */ append = c->argv[2]; - totlen = stringObjectLen(o)+sdslen((sds)ptrFromObj(append)); - if (checkStringLength(c,totlen) != C_OK) + if (checkStringLength(c,stringObjectLen(o),sdslen((sds)ptrFromObj(append))) != C_OK) return; /* Append the value */ diff --git a/src/t_zset.cpp b/src/t_zset.cpp index ad4ea2bd4..4d351da45 100644 --- a/src/t_zset.cpp +++ b/src/t_zset.cpp @@ -4214,8 +4214,13 @@ void zrandmemberCommand(client *c) { if (c->argc > 4 || (c->argc == 4 && strcasecmp(szFromObj(c->argv[3]),"withscores"))) { addReplyErrorObject(c,shared.syntaxerr); return; - } else if (c->argc == 4) + } else if (c->argc == 4) { withscores = 1; + if (l < LONG_MIN/2 || l > LONG_MAX/2) { + addReplyError(c,"value is out of range"); + return; + } + } zrandmemberWithCountCommand(c, l, withscores); return; } diff --git a/tests/support/test.tcl b/tests/support/test.tcl index d5a7f9dc5..b814758e3 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -40,6 +40,12 @@ proc assert_failed {expected_err detail} { error "assertion:$expected_err $detail" } +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} { assert_failed "Expected '$value' to be equal to '$expected'" $detail 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/hash.tcl b/tests/unit/type/hash.tcl index e95fd9fce..091b1cee1 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -68,6 +68,11 @@ start_server {tags {"hash"}} { r hrandfield myhash 0 } {} + test "HRANDFIELD count overflow" { + r hmset myhash a 1 + assert_error {*value is out of range*} {r hrandfield myhash -9223372036854770000 withvalues} + } {} + test "HRANDFIELD with against non existing key" { r hrandfield nonexisting_key 100 } {} diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 43968b26b..0c957906d 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -574,4 +574,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" + } + } + } } diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index 94b2ab480..5ea619ea3 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -1714,6 +1714,11 @@ start_server {tags {"zset"}} { r zrandmember nonexisting_key 100 } {} + test "ZRANDMEMBER count overflow" { + r zadd myzset 0 a + assert_error {*value is out of range*} {r zrandmember myzset -9223372036854770000 withscores} + } {} + # Make sure we can distinguish between an empty array and a null response r readraw 1