zhuyijun/read-redis
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Sanitize dump payload: fix empty keys when RDB loading and restore command (#9297)

Browse Source 
When we load rdb or restore command, if we encounter a length of 0, it will result in the creation of an empty key.
This could either be a corrupt payload, or a result of a bug (see #8453 )

This PR mainly fixes the following:
1) When restore command will return `Bad data format` error.
2) When loading RDB, we will silently discard the key.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 8ea777a6a02cae22aeff95f054d810f30b7b69ad)
This commit is contained in:
sundb 2021-08-06 03:42:20 +08:00 committed by Oran Agra
parent e34f06ae5d
commit 2f54107289
6 changed files with 127 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/cluster.c
View File

@ -5177,7 +5177,7 @@ void restoreCommand(client *c) {
rioInitWithBuffer(&payload,c->argv[3]->ptr);
if (((type = rdbLoadObjectType(&payload)) == -1) ||
((obj = rdbLoadObject(type,&payload,key->ptr)) == NULL))
((obj = rdbLoadObject(type,&payload,key->ptr,NULL)) == NULL))
{
addReplyError(c,"Bad data format");
return;

69
src/rdb.c
View File

@ -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) {
* 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 *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) {
} 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) {
} 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. */
size_t max_entries = server.set_max_intset_entries;
@ -1629,6 +1636,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
zset *zs;
if ((zsetlen = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL;
if (zsetlen == 0) goto emptykey;
o = createZsetObject();
zs = o->ptr;
@ -1691,6 +1700,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
len = rdbLoadLen(rdb, NULL);
if (len == RDB_LENERR) return NULL;
if (len == 0) goto emptykey;
o = createHashObject();
@ -1807,6 +1817,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
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);
@ -1940,6 +1952,13 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
}
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;
@ -1954,6 +1973,13 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
}
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;
@ -2250,7 +2276,12 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
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
@ -2351,6 +2382,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;
@ -2552,10 +2585,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)) == NULL) {
sdsfree(key);
goto eoferr;
}
val = rdbLoadObject(type,rdb,key,&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
@ -2565,18 +2595,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.
@ -2633,6 +2678,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():

7
src/rdb.h
View File

@ -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);
size_t rdbSavedObjectLen(robj *o, robj *key);
robj *rdbLoadObject(int type, rio *rdb, sds key);
robj *rdbLoadObject(int type, rio *rdb, sds key, int *error);
void backgroundSaveDoneHandler(int exitcode, int bysignal);
int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime);
ssize_t rdbSaveSingleModuleAux(rio *rdb, int when, moduleType *mt);

2
src/redis-check-rdb.c
View File

@ -308,7 +308,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)) == NULL) goto eoferr;
if ((val = rdbLoadObject(type,&rdb,key->ptr,NULL)) == NULL) goto eoferr;
/* Check if the key already expired. */
if (expiretime != -1 && expiretime < now)
rdbstate.already_expired++;

BIN
tests/assets/corrupt_empty_keys.rdb Normal file
View File

Binary file not shown.

65
tests/integration/corrupt-dump.tcl
View File

@ -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