diff options
| -rw-r--r-- | src/cluster.c | 2 | ||||
| -rw-r--r-- | src/rdb.c | 69 | ||||
| -rw-r--r-- | src/rdb.h | 7 | ||||
| -rw-r--r-- | src/redis-check-rdb.c | 2 | ||||
| -rw-r--r-- | tests/assets/corrupt_empty_keys.rdb | bin | 0 -> 187 bytes | |||
| -rw-r--r-- | tests/integration/corrupt-dump.tcl | 65 |
6 files changed, 127 insertions, 18 deletions
diff --git a/src/cluster.c b/src/cluster.c index 8cd931975..9bc33f493 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -5227,7 +5227,7 @@ void restoreCommand(client *c) { rioInitWithBuffer(&payload,c->argv[3]->ptr); if (((type = rdbLoadObjectType(&payload)) == -1) || - ((obj = rdbLoadObject(type,&payload,key->ptr,c->db->id)) == NULL)) + ((obj = rdbLoadObject(type,&payload,key->ptr,c->db->id,NULL)) == NULL)) { addReplyError(c,"Bad data format"); return; @@ -1516,12 +1516,17 @@ robj *rdbLoadCheckModuleValue(rio *rdb, char *modulename) { } /* Load a Redis object of the specified type from the specified file. - * On success a newly allocated object is returned, otherwise NULL. */ -robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { + * On success a newly allocated object is returned, otherwise NULL. + * When the function returns NULL and if 'error' is not NULL, the + * integer pointed by 'error' is set to the type of error that occurred */ +robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { robj *o = NULL, *ele, *dec; uint64_t len; unsigned int i; + /* Set default error of load object, it will be set to 0 on success. */ + if (error) *error = RDB_LOAD_ERR_OTHER; + int deep_integrity_validation = server.sanitize_dump_payload == SANITIZE_DUMP_YES; if (server.sanitize_dump_payload == SANITIZE_DUMP_CLIENTS) { /* Skip sanitization when loading (an RDB), or getting a RESTORE command @@ -1540,6 +1545,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { } else if (rdbtype == RDB_TYPE_LIST) { /* Read list value */ if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; + if (len == 0) goto emptykey; o = createQuicklistObject(); quicklistSetOptions(o->ptr, server.list_max_ziplist_size, @@ -1560,6 +1566,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { } else if (rdbtype == RDB_TYPE_SET) { /* Read Set value */ if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; + if (len == 0) goto emptykey; /* Use a regular set when there are too many entries. */ if (len > server.set_max_intset_entries) { @@ -1627,6 +1634,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { zset *zs; if ((zsetlen = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; + if (zsetlen == 0) goto emptykey; + o = createZsetObject(); zs = o->ptr; @@ -1685,6 +1694,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { len = rdbLoadLen(rdb, NULL); if (len == RDB_LENERR) return NULL; + if (len == 0) goto emptykey; o = createHashObject(); @@ -1792,6 +1802,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { serverAssert(len == 0); } else if (rdbtype == RDB_TYPE_LIST_QUICKLIST) { if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; + if (len == 0) goto emptykey; + o = createQuicklistObject(); quicklistSetOptions(o->ptr, server.list_max_ziplist_size, server.list_compress_depth); @@ -1923,6 +1935,13 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { } o->type = OBJ_ZSET; o->encoding = OBJ_ENCODING_ZIPLIST; + if (zsetLength(o) == 0) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } + if (zsetLength(o) > server.zset_max_ziplist_entries) zsetConvert(o,OBJ_ENCODING_SKIPLIST); break; @@ -1937,6 +1956,13 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { } o->type = OBJ_HASH; o->encoding = OBJ_ENCODING_ZIPLIST; + if (hashTypeLength(o) == 0) { + zfree(encoded); + o->ptr = NULL; + decrRefCount(o); + goto emptykey; + } + if (hashTypeLength(o) > server.hash_max_ziplist_entries) hashTypeConvert(o, OBJ_ENCODING_HT); break; @@ -2233,7 +2259,12 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid) { rdbReportReadError("Unknown RDB encoding type %d",rdbtype); return NULL; } + if (error) *error = 0; return o; + +emptykey: + if (error) *error = RDB_LOAD_ERR_EMPTY_KEY; + return NULL; } /* Mark that we are loading in the global state and setup the fields @@ -2334,6 +2365,8 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { int type, rdbver; redisDb *db = server.db+0; char buf[1024]; + int error; + long long empty_keys_skipped = 0, expired_keys_skipped = 0, keys_loaded = 0; rdb->update_cksum = rdbLoadProgressCallback; rdb->max_processing_chunk = server.loading_process_events_interval_bytes; @@ -2535,10 +2568,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { if ((key = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) goto eoferr; /* Read value */ - if ((val = rdbLoadObject(type,rdb,key,db->id)) == NULL) { - sdsfree(key); - goto eoferr; - } + val = rdbLoadObject(type,rdb,key,db->id,&error); /* Check if the key already expired. This function is used when loading * an RDB file from disk, either at startup, or when an RDB was @@ -2548,18 +2578,33 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { * Similarly if the RDB is the preamble of an AOF file, we want to * load all the keys as they are, since the log of operations later * assume to work in an exact keyspace state. */ - if (iAmMaster() && + if (val == NULL) { + /* Since we used to have bug that could lead to empty keys + * (See #8453), we rather not fail when empty key is encountered + * in an RDB file, instead we will silently discard it and + * continue loading. */ + if (error == RDB_LOAD_ERR_EMPTY_KEY) { + if(empty_keys_skipped++ < 10) + serverLog(LL_WARNING, "rdbLoadObject skipping empty key: %s", key); + sdsfree(key); + } else { + sdsfree(key); + goto eoferr; + } + } else if (iAmMaster() && !(rdbflags&RDBFLAGS_AOF_PREAMBLE) && expiretime != -1 && expiretime < now) { sdsfree(key); decrRefCount(val); + expired_keys_skipped++; } else { robj keyobj; initStaticStringObject(keyobj,key); /* Add the new object in the hash table */ int added = dbAddRDBLoad(db,key,val); + keys_loaded++; if (!added) { if (rdbflags & RDBFLAGS_ALLOW_DUP) { /* This flag is useful for DEBUG RELOAD special modes. @@ -2616,6 +2661,16 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { } } } + + if (empty_keys_skipped) { + serverLog(LL_WARNING, + "Done loading RDB, keys loaded: %lld, keys expired: %lld, empty keys skipped: %lld.", + keys_loaded, expired_keys_skipped, empty_keys_skipped); + } else { + serverLog(LL_WARNING, + "Done loading RDB, keys loaded: %lld, keys expired: %lld.", + keys_loaded, expired_keys_skipped); + } return C_OK; /* Unexpected end of file is handled here calling rdbReportReadError(): @@ -127,6 +127,11 @@ #define RDBFLAGS_REPLICATION (1<<1) /* Load/save for SYNC. */ #define RDBFLAGS_ALLOW_DUP (1<<2) /* Allow duplicated keys when loading.*/ +/* When rdbLoadObject() returns NULL, the err flag is + * set to hold the type of error that occurred */ +#define RDB_LOAD_ERR_EMPTY_KEY 1 /* Error of empty key */ +#define RDB_LOAD_ERR_OTHER 2 /* Any other errors */ + int rdbSaveType(rio *rdb, unsigned char type); int rdbLoadType(rio *rdb); int rdbSaveTime(rio *rdb, time_t t); @@ -145,7 +150,7 @@ void rdbRemoveTempFile(pid_t childpid, int from_signal); int rdbSave(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); +robj *rdbLoadObject(int type, 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/src/redis-check-rdb.c b/src/redis-check-rdb.c index b2cc6a5b4..82429bb25 100644 --- a/src/redis-check-rdb.c +++ b/src/redis-check-rdb.c @@ -310,7 +310,7 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { rdbstate.keys++; /* Read value */ rdbstate.doing = RDB_CHECK_DOING_READ_OBJECT_VALUE; - if ((val = rdbLoadObject(type,&rdb,key->ptr,selected_dbid)) == NULL) goto eoferr; + if ((val = rdbLoadObject(type,&rdb,key->ptr,selected_dbid,NULL)) == NULL) goto eoferr; /* Check if the key already expired. */ if (expiretime != -1 && expiretime < now) rdbstate.already_expired++; diff --git a/tests/assets/corrupt_empty_keys.rdb b/tests/assets/corrupt_empty_keys.rdb Binary files differnew file mode 100644 index 000000000..bc2b4f202 --- /dev/null +++ b/tests/assets/corrupt_empty_keys.rdb diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index c707d3410..b4af173f1 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -148,6 +148,23 @@ 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 + + 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 + } +} + test {corrupt payload: listpack invalid size header} { start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { r config set sanitize-dump-payload no @@ -328,12 +345,13 @@ test {corrupt payload: fuzzer findings - leak in rdbloading due to dup entry in } } -test {corrupt payload: fuzzer findings - empty intset div by zero} { +test {corrupt payload: fuzzer findings - empty intset} { 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 - r RESTORE _setbig 0 "\x02\xC0\xC0\x06\x02\x5F\x39\xC0\x02\x02\x5F\x33\xC0\x00\x02\x5F\x31\xC0\x04\xC0\x08\x02\x5F\x37\x02\x5F\x35\x09\x00\xC5\xD4\x6D\xBA\xAD\x14\xB7\xE7" - catch {r SRANDMEMBER _setbig } + catch {r RESTORE _setbig 0 "\x02\xC0\xC0\x06\x02\x5F\x39\xC0\x02\x02\x5F\x33\xC0\x00\x02\x5F\x31\xC0\x04\xC0\x08\x02\x5F\x37\x02\x5F\x35\x09\x00\xC5\xD4\x6D\xBA\xAD\x14\xB7\xE7"} err + assert_match "*Bad data format*" $err + r ping } } @@ -507,14 +525,13 @@ test {corrupt payload: fuzzer findings - valgrind invalid read} { } } -test {corrupt payload: fuzzer findings - HRANDFIELD on bad ziplist} { +test {corrupt payload: fuzzer findings - empty hash ziplist} { start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { r config set sanitize-dump-payload yes r debug set-skip-checksum-validation 1 - r RESTORE _int 0 "\x04\xC0\x01\x09\x00\xF6\x8A\xB6\x7A\x85\x87\x72\x4D" - catch {r HRANDFIELD _int} - assert_equal [count_log_message 0 "crashed by signal"] 0 - assert_equal [count_log_message 0 "ASSERTION FAILED"] 1 + catch {r RESTORE _int 0 "\x04\xC0\x01\x09\x00\xF6\x8A\xB6\x7A\x85\x87\x72\x4D"} err + assert_match "*Bad data format*" $err + r ping } } @@ -529,5 +546,37 @@ test {corrupt payload: fuzzer findings - stream with no records} { } } +test {corrupt payload: fuzzer findings - empty quicklist} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + r debug set-skip-checksum-validation 1 + catch { + r restore key 0 "\x0E\xC0\x2B\x15\x00\x00\x00\x0A\x00\x00\x00\x01\x00\x00\xE0\x62\x58\xEA\xDF\x22\x00\x00\x00\xFF\x09\x00\xDF\x35\xD2\x67\xDC\x0E\x89\xAB" replace + } err + assert_match "*Bad data format*" $err + r ping + } +} + +test {corrupt payload: fuzzer findings - empty zset} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + r debug set-skip-checksum-validation 1 + catch {r restore key 0 "\x05\xC0\x01\x09\x00\xF6\x8A\xB6\x7A\x85\x87\x72\x4D"} err + assert_match "*Bad data format*" $err + r ping + } +} + +test {corrupt payload: fuzzer findings - hash with len of 0} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r config set sanitize-dump-payload yes + r debug set-skip-checksum-validation 1 + catch {r restore key 0 "\x04\xC0\x21\x09\x00\xF6\x8A\xB6\x7A\x85\x87\x72\x4D"} err + assert_match "*Bad data format*" $err + r ping + } +} + } ;# tags |