diff --git a/src/rdb.c b/src/rdb.c index b9ce53c57..111398b4f 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1827,7 +1827,19 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { zfree(zl); 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 || rdbtype == RDB_TYPE_LIST_ZIPLIST || @@ -1910,6 +1922,14 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { decrRefCount(o); return NULL; } + + if (ziplistLen(encoded) == 0) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } + o->type = OBJ_LIST; o->encoding = OBJ_ENCODING_ZIPLIST; listTypeConvert(o,OBJ_ENCODING_QUICKLIST); diff --git a/src/ziplist.c b/src/ziplist.c index 1b934a292..b55c85fcf 100644 --- a/src/ziplist.c +++ b/src/ziplist.c @@ -1527,7 +1527,7 @@ int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep, return 0; /* Make sure the 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; /* Check that the count in the header is correct */ diff --git a/src/zipmap.c b/src/zipmap.c index e39c27708..22cf17cbb 100644 --- a/src/zipmap.c +++ b/src/zipmap.c @@ -430,6 +430,9 @@ int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) { return 0; } + /* check that the zipmap is not empty. */ + if (count == 0) return 0; + /* check that the count in the header is correct */ if (zm[0] != ZIPMAP_BIGLEN && zm[0] != count) return 0; diff --git a/tests/assets/corrupt_empty_keys.rdb b/tests/assets/corrupt_empty_keys.rdb index bc2b4f202..8f260d493 100644 Binary files a/tests/assets/corrupt_empty_keys.rdb and b/tests/assets/corrupt_empty_keys.rdb differ diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index 554de609e..468082b91 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -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} { 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 @@ -148,20 +158,24 @@ test {corrupt payload: load corrupted rdb with no CRC - #3505} { kill_server $srv ;# let valgrind look for issues } -test {corrupt payload: load corrupted rdb with empty keys} { - set server_path [tmpdir "server.rdb-corruption-empty-keys-test"] - exec cp tests/assets/corrupt_empty_keys.rdb $server_path - start_server [list overrides [list "dir" $server_path "dbfilename" "corrupt_empty_keys.rdb"]] { - r select 0 - assert_equal [r dbsize] 0 +foreach sanitize_dump {no yes} { + test {corrupt payload: load corrupted rdb with empty keys} { + set server_path [tmpdir "server.rdb-corruption-empty-keys-test"] + exec cp tests/assets/corrupt_empty_keys.rdb $server_path + start_server [list overrides [list "dir" $server_path "dbfilename" "corrupt_empty_keys.rdb" "sanitize-dump-payload" $sanitize_dump]] { + 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: hash*" 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: 6*" 0 + verify_log_message 0 "*skipping empty key: set*" 0 + verify_log_message 0 "*skipping empty key: list_quicklist*" 0 + verify_log_message 0 "*skipping empty key: list_quicklist_empty_ziplist*" 0 + verify_log_message 0 "*skipping empty key: list_ziplist*" 0 + verify_log_message 0 "*skipping empty key: hash*" 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} { } } +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} { start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { r config set sanitize-dump-payload no