slipenkomk/valkey
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix possible crash in command getkeys (#12380)

Browse Source 
When getKeysUsingKeySpecs processes a command with more than one key-spec,
and called with a total of more than 256 keys, it'll call getKeysPrepareResult again,
but since numkeys isn't updated, getKeysPrepareResult will not bother to copy key
names from the old result (leaving these slots uninitialized). Furthermore, it did not
consider the keys it already found when allocating more space.

Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
Lior Lahav 2023-07-03 12:45:18 +03:00 committed by GitHub
parent 6b57241fa8
commit b7559d9f3b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/db.c
View File

@ -1871,8 +1871,9 @@ int64_t getAllKeySpecsFlags(struct redisCommand *cmd, int inv) {
* found in other valid keyspecs.
*/
int getKeysUsingKeySpecs(struct redisCommand *cmd, robj **argv, int argc, int search_flags, getKeysResult *result) {
int j, i, k = 0, last, first, step;
int j, i, last, first, step;
keyReference *keys;
result->numkeys = 0;
for (j = 0; j < cmd->key_specs_num; j++) {
keySpec *spec = cmd->key_specs + j;
@ -1937,7 +1938,7 @@ int getKeysUsingKeySpecs(struct redisCommand *cmd, robj **argv, int argc, int se
}
int count = ((last - first)+1);
keys = getKeysPrepareResult(result, count);
keys = getKeysPrepareResult(result, result->numkeys + count);
/* First or last is out of bounds, which indicates a syntax error */
if (last >= argc || last < first || first >= argc) {
@ -1958,8 +1959,9 @@ int getKeysUsingKeySpecs(struct redisCommand *cmd, robj **argv, int argc, int se
serverPanic("Redis built-in command declared keys positions not matching the arity requirements.");
}
}
keys[k].pos = i;
keys[k++].flags = spec->flags;
keys[result->numkeys].pos = i;
keys[result->numkeys].flags = spec->flags;
result->numkeys++;
}
/* Handle incomplete specs (only after we added the current spec
@ -1980,8 +1982,7 @@ invalid_spec:
}
}
result->numkeys = k;
return k;
return result->numkeys;
}
/* Return all the arguments that are keys in the command passed via argc / argv.

15
tests/unit/introspection-2.tcl
View File

@ -153,6 +153,21 @@ start_server {tags {"introspection"}} {
assert_equal {key1 key2} [r command getkeys lcs key1 key2]
}
test {COMMAND GETKEYS MORE THAN 256 KEYS} {
set all_keys [list]
set numkeys 260
for {set i 1} {$i <= $numkeys} {incr i} {
lappend all_keys "key$i"
}
set all_keys_with_target [linsert $all_keys 0 target]
# we are using ZUNIONSTORE command since in order to reproduce allocation of a new buffer in getKeysPrepareResult
# when numkeys in result > 0
# we need a command that the final number of keys is not known in the first call to getKeysPrepareResult
# before the fix in that case data of old buffer was not copied to the new result buffer
# causing all previous keys (numkeys) data to be uninitialize
assert_equal $all_keys_with_target [r command getkeys ZUNIONSTORE target $numkeys {*}$all_keys]
}
test "COMMAND LIST syntax error" {
assert_error "ERR syntax error*" {r command list bad_arg}
assert_error "ERR syntax error*" {r command list filterby bad_arg}