Unverified Commit cbda4929 authored by sundb's avatar sundb Committed by GitHub
Browse files

Sanitize dump payload: handle remaining empty key when RDB loading and restore command (#9349)

This commit mainly fixes empty keys due to RDB loading and restore command,
which was omitted in #9297.

1) When loading quicklsit, if all the ziplists in the quicklist are empty, NULL will be returned.
    If only some of the ziplists are empty, then we will skip the empty ziplists silently.
2) When loading hash zipmap, if zipmap is empty, sanitization check will fail.
3) When loading hash ziplist, if ziplist is empty, NULL will be returned.
4) Add RDB loading test with sanitize.
parent 0b643e93
...@@ -1827,7 +1827,19 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { ...@@ -1827,7 +1827,19 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
zfree(zl); zfree(zl);
return NULL; return NULL;
} }
quicklistAppendZiplist(o->ptr, zl);
/* Silently skip empty ziplists, if we'll end up with empty quicklist we'll fail later. */
if (ziplistLen(zl) == 0) {
zfree(zl);
continue;
} else {
quicklistAppendZiplist(o->ptr, zl);
}
}
if (quicklistCount(o->ptr) == 0) {
decrRefCount(o);
goto emptykey;
} }
} else if (rdbtype == RDB_TYPE_HASH_ZIPMAP || } else if (rdbtype == RDB_TYPE_HASH_ZIPMAP ||
rdbtype == RDB_TYPE_LIST_ZIPLIST || rdbtype == RDB_TYPE_LIST_ZIPLIST ||
...@@ -1910,6 +1922,14 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { ...@@ -1910,6 +1922,14 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
decrRefCount(o); decrRefCount(o);
return NULL; return NULL;
} }
if (ziplistLen(encoded) == 0) {
zfree(encoded);
o->ptr = NULL;
decrRefCount(o);
goto emptykey;
}
o->type = OBJ_LIST; o->type = OBJ_LIST;
o->encoding = OBJ_ENCODING_ZIPLIST; o->encoding = OBJ_ENCODING_ZIPLIST;
listTypeConvert(o,OBJ_ENCODING_QUICKLIST); listTypeConvert(o,OBJ_ENCODING_QUICKLIST);
......
...@@ -1527,7 +1527,7 @@ int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep, ...@@ -1527,7 +1527,7 @@ int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep,
return 0; return 0;
/* Make sure the <zltail> entry really do point to the start of the last entry. */ /* Make sure the <zltail> entry really do point to the start of the last entry. */
if (prev != ZIPLIST_ENTRY_TAIL(zl)) if (prev != NULL && prev != ZIPLIST_ENTRY_TAIL(zl))
return 0; return 0;
/* Check that the count in the header is correct */ /* Check that the count in the header is correct */
......
...@@ -430,6 +430,9 @@ int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) { ...@@ -430,6 +430,9 @@ int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) {
return 0; return 0;
} }
/* check that the zipmap is not empty. */
if (count == 0) return 0;
/* check that the count in the header is correct */ /* check that the count in the header is correct */
if (zm[0] != ZIPMAP_BIGLEN && zm[0] != count) if (zm[0] != ZIPMAP_BIGLEN && zm[0] != count)
return 0; return 0;
......
...@@ -118,6 +118,16 @@ test {corrupt payload: #3080 - quicklist} { ...@@ -118,6 +118,16 @@ test {corrupt payload: #3080 - quicklist} {
} }
} }
test {corrupt payload: quicklist with empty ziplist} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
r debug set-skip-checksum-validation 1
catch {r restore key 0 "\x0E\x01\x0B\x0B\x00\x00\x00\x0A\x00\x00\x00\x00\x00\xFF\x09\x00\xC2\x69\x37\x83\x3C\x7F\xFE\x6F" replace} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: #3080 - ziplist} { test {corrupt payload: #3080 - ziplist} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
# shallow sanitization is enough for restore to safely reject the payload with wrong size # shallow sanitization is enough for restore to safely reject the payload with wrong size
...@@ -148,20 +158,24 @@ test {corrupt payload: load corrupted rdb with no CRC - #3505} { ...@@ -148,20 +158,24 @@ test {corrupt payload: load corrupted rdb with no CRC - #3505} {
kill_server $srv ;# let valgrind look for issues kill_server $srv ;# let valgrind look for issues
} }
test {corrupt payload: load corrupted rdb with empty keys} { foreach sanitize_dump {no yes} {
set server_path [tmpdir "server.rdb-corruption-empty-keys-test"] test {corrupt payload: load corrupted rdb with empty keys} {
exec cp tests/assets/corrupt_empty_keys.rdb $server_path set server_path [tmpdir "server.rdb-corruption-empty-keys-test"]
start_server [list overrides [list "dir" $server_path "dbfilename" "corrupt_empty_keys.rdb"]] { exec cp tests/assets/corrupt_empty_keys.rdb $server_path
r select 0 start_server [list overrides [list "dir" $server_path "dbfilename" "corrupt_empty_keys.rdb" "sanitize-dump-payload" $sanitize_dump]] {
assert_equal [r dbsize] 0 r select 0
assert_equal [r dbsize] 0
verify_log_message 0 "*skipping empty key: set*" 0
verify_log_message 0 "*skipping empty key: quicklist*" 0 verify_log_message 0 "*skipping empty key: set*" 0
verify_log_message 0 "*skipping empty key: hash*" 0 verify_log_message 0 "*skipping empty key: list_quicklist*" 0
verify_log_message 0 "*skipping empty key: hash_ziplist*" 0 verify_log_message 0 "*skipping empty key: list_quicklist_empty_ziplist*" 0
verify_log_message 0 "*skipping empty key: zset*" 0 verify_log_message 0 "*skipping empty key: list_ziplist*" 0
verify_log_message 0 "*skipping empty key: zset_ziplist*" 0 verify_log_message 0 "*skipping empty key: hash*" 0
verify_log_message 0 "*empty keys skipped: 6*" 0 verify_log_message 0 "*skipping empty key: hash_ziplist*" 0
verify_log_message 0 "*skipping empty key: zset*" 0
verify_log_message 0 "*skipping empty key: zset_ziplist*" 0
verify_log_message 0 "*empty keys skipped: 8*" 0
}
} }
} }
...@@ -241,6 +255,16 @@ test {corrupt payload: hash duplicate records} { ...@@ -241,6 +255,16 @@ test {corrupt payload: hash duplicate records} {
} }
} }
test {corrupt payload: hash empty zipmap} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
r debug set-skip-checksum-validation 1
catch { r RESTORE _hash 0 "\x09\x02\x00\xFF\x09\x00\xC0\xF1\xB8\x67\x4C\x16\xAC\xE3" } err
assert_match "*Bad data format*" $err
verify_log_message 0 "*Zipmap integrity check failed*" 0
}
}
test {corrupt payload: fuzzer findings - NPD in streamIteratorGetID} { test {corrupt payload: fuzzer findings - NPD in streamIteratorGetID} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no r config set sanitize-dump-payload no
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment