From 51887e61b8a0ae36fa977351f34ace3e38efbc29 Mon Sep 17 00:00:00 2001 From: Binbin Date: Sun, 20 Nov 2022 18:12:15 +0800 Subject: [PATCH] sanitize dump payload: fix crash with empty set with listpack encoding (#11519) The following example will create an empty set (listpack encoding): ``` > RESTORE key 0 "\x14\x25\x25\x00\x00\x00\x00\x00\x02\x01\x82\x5F\x37\x03\x06\x01\x82\x5F\x35\x03\x82\x5F\x33\x03\x00\x01\x82\x5F\x31\x03\x82\x5F\x39\x03\x04\xA9\x08\x01\xFF\x0B\x00\xA3\x26\x49\xB4\x86\xB0\x0F\x41" OK > SCARD key (integer) 0 > SRANDMEMBER key Error: Server closed the connection ``` In the spirit of #9297, skip empty set when loading RDB_TYPE_SET_LISTPACK. Introduced in #11290 --- src/rdb.c | 7 +++++++ src/rdb.h | 2 +- tests/integration/corrupt-dump-fuzzer.tcl | 3 ++- tests/integration/corrupt-dump.tcl | 9 +++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index 5118c17b4..3bb8a4cc8 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2319,6 +2319,13 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { } o->type = OBJ_SET; o->encoding = OBJ_ENCODING_LISTPACK; + + if (setTypeSize(o) == 0) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } if (setTypeSize(o) > server.set_max_listpack_entries) setTypeConvert(o, OBJ_ENCODING_HT); break; diff --git a/src/rdb.h b/src/rdb.h index 4350d3a28..b8c25865c 100644 --- a/src/rdb.h +++ b/src/rdb.h @@ -158,7 +158,7 @@ void rdbRemoveTempFile(pid_t childpid, int from_signal); int rdbSave(int req, char *filename, rdbSaveInfo *rsi); ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid); size_t rdbSavedObjectLen(robj *o, robj *key, int dbid); -robj *rdbLoadObject(int type, rio *rdb, sds key, int dbid, int *error); +robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error); void backgroundSaveDoneHandler(int exitcode, int bysignal); int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime,int dbid); ssize_t rdbSaveSingleModuleAux(rio *rdb, int when, moduleType *mt); diff --git a/tests/integration/corrupt-dump-fuzzer.tcl b/tests/integration/corrupt-dump-fuzzer.tcl index 011c314b4..7c3b90969 100644 --- a/tests/integration/corrupt-dump-fuzzer.tcl +++ b/tests/integration/corrupt-dump-fuzzer.tcl @@ -1,4 +1,4 @@ -# tests of corrupt ziplist payload with valid CRC +# tests of corrupt listpack payload with valid CRC tags {"dump" "corruption" "external:skip"} { @@ -32,6 +32,7 @@ proc generate_collections {suffix elements} { proc generate_types {} { r config set list-max-ziplist-size 5 r config set hash-max-ziplist-entries 5 + r config set set-max-listpack-entries 5 r config set zset-max-ziplist-entries 5 r config set stream-node-max-entries 5 diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index 1b9be676b..ef878457a 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -800,6 +800,15 @@ test {corrupt payload: fuzzer findings - valgrind fishy value warning} { } } +test {corrupt payload: fuzzer findings - empty set listpack} { + 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 "\x14\x25\x25\x00\x00\x00\x00\x00\x02\x01\x82\x5F\x37\x03\x06\x01\x82\x5F\x35\x03\x82\x5F\x33\x03\x00\x01\x82\x5F\x31\x03\x82\x5F\x39\x03\x04\xA9\x08\x01\xFF\x0B\x00\xA3\x26\x49\xB4\x86\xB0\x0F\x41"} err + assert_match "*Bad data format*" $err + r ping + } +} } ;# tags