diff --git a/src/dict.c b/src/dict.c index 21c616e6f..1bbdc211e 100644 --- a/src/dict.c +++ b/src/dict.c @@ -154,6 +154,10 @@ int _dictExpand(dict *d, unsigned long size, int* malloc_failed) dictht n; /* the new hash table */ unsigned long realsize = _dictNextPower(size); + /* Detect overflows */ + if (realsize < size || realsize * sizeof(dictEntry*) < realsize) + return DICT_ERR; + /* Rehashing to the same table size is not useful. */ if (realsize == d->ht[0].size) return DICT_ERR; diff --git a/src/listpack.c b/src/listpack.c index 27622d4a5..8424da87f 100644 --- a/src/listpack.c +++ b/src/listpack.c @@ -131,6 +131,8 @@ assert((p) >= (lp)+LP_HDR_SIZE && (p)+(len) < (lp)+lpGetTotalBytes((lp))); \ } while (0) +static inline void lpAssertValidEntry(unsigned char* lp, size_t lpbytes, unsigned char *p); + /* Convert a string into a signed 64 bit integer. * The function returns 1 if the string could be parsed into a (non-overflowing) * signed 64 bit int, 0 otherwise. The 'value' will be set to the parsed value @@ -453,8 +455,8 @@ unsigned char *lpSkip(unsigned char *p) { unsigned char *lpNext(unsigned char *lp, unsigned char *p) { assert(p); p = lpSkip(p); - ASSERT_INTEGRITY(lp, p); if (p[0] == LP_EOF) return NULL; + lpAssertValidEntry(lp, lpBytes(lp), p); return p; } @@ -468,16 +470,17 @@ unsigned char *lpPrev(unsigned char *lp, unsigned char *p) { uint64_t prevlen = lpDecodeBacklen(p); prevlen += lpEncodeBacklen(NULL,prevlen); p -= prevlen-1; /* Seek the first byte of the previous entry. */ - ASSERT_INTEGRITY(lp, p); + lpAssertValidEntry(lp, lpBytes(lp), p); return p; } /* Return a pointer to the first element of the listpack, or NULL if the * listpack has no elements. */ unsigned char *lpFirst(unsigned char *lp) { - lp += LP_HDR_SIZE; /* Skip the header. */ - if (lp[0] == LP_EOF) return NULL; - return lp; + unsigned char *p = lp + LP_HDR_SIZE; /* Skip the header. */ + if (p[0] == LP_EOF) return NULL; + lpAssertValidEntry(lp, lpBytes(lp), p); + return p; } /* Return a pointer to the last element of the listpack, or NULL if the @@ -861,6 +864,13 @@ unsigned char *lpSeek(unsigned char *lp, long index) { } } +/* Same as lpFirst but without validation assert, to be used right before lpValidateNext. */ +unsigned char *lpValidateFirst(unsigned char *lp) { + unsigned char *p = lp + LP_HDR_SIZE; /* Skip the header. */ + if (p[0] == LP_EOF) return NULL; + return p; +} + /* Validate the integrity of a single listpack entry and move to the next one. * The input argument 'pp' is a reference to the current record and is advanced on exit. * Returns 1 if valid, 0 if invalid. */ @@ -872,6 +882,10 @@ int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes) { if (!p) return 0; + /* Before accessing p, make sure it's valid. */ + if (OUT_OF_RANGE(p)) + return 0; + if (*p == LP_EOF) { *pp = NULL; return 1; @@ -908,6 +922,11 @@ int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes) { #undef OUT_OF_RANGE } +/* Validate that the entry doesn't reach outside the listpack allocation. */ +static inline void lpAssertValidEntry(unsigned char* lp, size_t lpbytes, unsigned char *p) { + assert(lpValidateNext(lp, &p, lpbytes)); +} + /* Validate the integrity of the data structure. * when `deep` is 0, only the integrity of the header is validated. * when `deep` is 1, we scan all the entries one by one. */ @@ -930,8 +949,8 @@ int lpValidateIntegrity(unsigned char *lp, size_t size, int deep){ /* Validate the invividual entries. */ uint32_t count = 0; - unsigned char *p = lpFirst(lp); - while(p) { + unsigned char *p = lp + LP_HDR_SIZE; + while(p && p[0] != LP_EOF) { if (!lpValidateNext(lp, &p, bytes)) return 0; count++; diff --git a/src/listpack.h b/src/listpack.h index f87622c18..9a5115bad 100644 --- a/src/listpack.h +++ b/src/listpack.h @@ -60,6 +60,7 @@ unsigned char *lpPrev(unsigned char *lp, unsigned char *p); uint32_t lpBytes(unsigned char *lp); unsigned char *lpSeek(unsigned char *lp, long index); int lpValidateIntegrity(unsigned char *lp, size_t size, int deep); +unsigned char *lpValidateFirst(unsigned char *lp); int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes); #endif diff --git a/src/object.c b/src/object.c index c6381a231..23920fb2c 100644 --- a/src/object.c +++ b/src/object.c @@ -123,6 +123,21 @@ robj *createStringObject(const char *ptr, size_t len) { return createRawStringObject(ptr,len); } +/* Same as CreateRawStringObject, can return NULL if allocation fails */ +robj *tryCreateRawStringObject(const char *ptr, size_t len) { + sds str = sdstrynewlen(ptr,len); + if (!str) return NULL; + return createObject(OBJ_STRING, str); +} + +/* Same as createStringObject, can return NULL if allocation fails */ +robj *tryCreateStringObject(const char *ptr, size_t len) { + if (len <= OBJ_ENCODING_EMBSTR_SIZE_LIMIT) + return createEmbeddedStringObject(ptr,len); + else + return tryCreateRawStringObject(ptr,len); +} + /* Create a string object from a long long value. When possible returns a * shared integer object, or at least an integer encoded one. * diff --git a/src/rdb.c b/src/rdb.c index 0fd19a944..051a0d87e 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -545,8 +545,12 @@ void *rdbGenericLoadStringObject(rio *rdb, int flags, size_t *lenptr) { } return buf; } else { - robj *o = encode ? createStringObject(SDS_NOINIT,len) : - createRawStringObject(SDS_NOINIT,len); + robj *o = encode ? tryCreateStringObject(SDS_NOINIT,len) : + tryCreateRawStringObject(SDS_NOINIT,len); + if (!o) { + serverLog(server.loading? LL_WARNING: LL_VERBOSE, "rdbGenericLoadStringObject failed allocating %llu bytes", len); + return NULL; + } if (len && rioRead(rdb,o->ptr,len) == 0) { decrRefCount(o); return NULL; @@ -2210,6 +2214,23 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int *error) { } } } + + /* Verify that each PEL eventually got a consumer assigned to it. */ + if (deep_integrity_validation) { + raxIterator ri_cg_pel; + raxStart(&ri_cg_pel,cgroup->pel); + raxSeek(&ri_cg_pel,"^",NULL,0); + while(raxNext(&ri_cg_pel)) { + streamNACK *nack = ri_cg_pel.data; + if (!nack->consumer) { + raxStop(&ri_cg_pel); + rdbReportCorruptRDB("Stream CG PEL entry without consumer"); + decrRefCount(o); + return NULL; + } + } + raxStop(&ri_cg_pel); + } } } else if (rdbtype == RDB_TYPE_MODULE || rdbtype == RDB_TYPE_MODULE_2) { uint64_t moduleid = rdbLoadLen(rdb,NULL); diff --git a/src/server.h b/src/server.h index abdbca8e5..74bd5447a 100644 --- a/src/server.h +++ b/src/server.h @@ -1973,6 +1973,8 @@ robj *createObject(int type, void *ptr); robj *createStringObject(const char *ptr, size_t len); robj *createRawStringObject(const char *ptr, size_t len); robj *createEmbeddedStringObject(const char *ptr, size_t len); +robj *tryCreateRawStringObject(const char *ptr, size_t len); +robj *tryCreateStringObject(const char *ptr, size_t len); robj *dupStringObject(const robj *o); int isSdsRepresentableAsLongLong(sds s, long long *llval); int isObjectRepresentableAsLongLong(robj *o, long long *llongval); diff --git a/src/t_stream.c b/src/t_stream.c index 574195ee3..7e67c1b0e 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -3590,7 +3590,7 @@ int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep) { /* In non-deep mode we just validated the listpack header (encoded size) */ if (!deep) return 1; - next = p = lpFirst(lp); + next = p = lpValidateFirst(lp); if (!lpValidateNext(lp, &next, size)) return 0; if (!p) return 0; @@ -3629,7 +3629,11 @@ int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep) { /* entry id */ p = next; if (!lpValidateNext(lp, &next, size)) return 0; + lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; p = next; if (!lpValidateNext(lp, &next, size)) return 0; + lpGetIntegerIfValid(p, &valid_record); + if (!valid_record) return 0; if (!(flags & STREAM_ITEM_FLAG_SAMEFIELDS)) { /* num-of-fields */ diff --git a/src/ziplist.c b/src/ziplist.c index fdc1bb9e1..74c033ce2 100644 --- a/src/ziplist.c +++ b/src/ziplist.c @@ -1537,6 +1537,10 @@ int ziplistValidateIntegrity(unsigned char *zl, size_t size, int deep, count++; } + /* Make sure 'p' really does point to the end of the ziplist. */ + if (p != zl + bytes - ZIPLIST_END_SIZE) + return 0; + /* Make sure the entry really do point to the start of the last entry. */ if (prev != ZIPLIST_ENTRY_TAIL(zl)) return 0; diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index 723ce1d64..896571c2f 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -546,6 +546,92 @@ test {corrupt payload: fuzzer findings - stream with no records} { } } +test {corrupt payload: fuzzer findings - quicklist ziplist tail followed by extra data which start with 0xff} { + 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\x01\x11\x11\x00\x00\x00\x0A\x00\x00\x00\x01\x00\x00\xF6\xFF\xB0\x6C\x9C\xFF\x09\x00\x9C\x37\x47\x49\x4D\xDE\x94\xF5" replace + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*integrity check failed*" 0 + } +} + +test {corrupt payload: fuzzer findings - dict init to huge size} { + 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 "\x02\x81\xC0\x00\x02\x5F\x31\xC0\x02\x09\x00\xB2\x1B\xE5\x17\x2E\x15\xF4\x6C" replace} err + assert_match "*Bad data format*" $err + r ping + } +} + +test {corrupt payload: fuzzer findings - huge string} { + 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 "\x00\x81\x01\x09\x00\xF6\x2B\xB6\x7A\x85\x87\x72\x4D"} err + assert_match "*Bad data format*" $err + r ping + } +} + +test {corrupt payload: fuzzer findings - stream PEL without consumer} { + 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 _stream 0 "\x0F\x01\x10\x00\x00\x01\x7B\x08\xF0\xB2\x34\x00\x00\x00\x00\x00\x00\x00\x00\xC3\x3B\x40\x42\x19\x42\x00\x00\x00\x18\x00\x02\x01\x01\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x00\x20\x10\x00\x00\x20\x01\x00\x01\x20\x03\x02\x05\x01\x03\x20\x05\x40\x00\x04\x82\x5F\x31\x03\x05\x60\x19\x80\x32\x02\x05\x01\xFF\x02\x81\x00\x00\x01\x7B\x08\xF0\xB2\x34\x02\x01\x07\x6D\x79\x67\x72\x6F\x75\x70\x81\x00\x00\x01\x7B\x08\xF0\xB2\x34\x01\x01\x00\x00\x01\x7B\x08\xF0\xB2\x34\x00\x00\x00\x00\x00\x00\x00\x01\x35\xB2\xF0\x08\x7B\x01\x00\x00\x01\x01\x13\x41\x6C\x69\x63\x65\x35\xB2\xF0\x08\x7B\x01\x00\x00\x01\x00\x00\x01\x7B\x08\xF0\xB2\x34\x00\x00\x00\x00\x00\x00\x00\x01\x09\x00\x28\x2F\xE0\xC5\x04\xBB\xA7\x31"} err + assert_match "*Bad data format*" $err + #catch {r XINFO STREAM _stream FULL } + r ping + } +} + +test {corrupt payload: fuzzer findings - stream listpack valgrind issue} { + 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 _stream 0 "\x0F\x01\x10\x00\x00\x01\x7B\x09\x5E\x94\xFF\x00\x00\x00\x00\x00\x00\x00\x00\x40\x42\x42\x00\x00\x00\x18\x00\x02\x01\x01\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x00\x01\x02\x01\x00\x01\x00\x01\x01\x01\x00\x01\x05\x01\x03\x01\x25\x01\x00\x01\x01\x01\x82\x5F\x31\x03\x05\x01\x02\x01\x32\x01\x00\x01\x01\x01\x02\x01\xF0\x01\xFF\x02\x81\x00\x00\x01\x7B\x09\x5E\x95\x31\x00\x01\x07\x6D\x79\x67\x72\x6F\x75\x70\x81\x00\x00\x01\x7B\x09\x5E\x95\x24\x00\x01\x00\x00\x01\x7B\x09\x5E\x95\x24\x00\x00\x00\x00\x00\x00\x00\x00\x5C\x95\x5E\x09\x7B\x01\x00\x00\x01\x01\x05\x41\x6C\x69\x63\x65\x4B\x95\x5E\x09\x7B\x01\x00\x00\x01\x00\x00\x01\x7B\x09\x5E\x95\x24\x00\x00\x00\x00\x00\x00\x00\x00\x09\x00\x19\x29\x94\xDF\x76\xF8\x1A\xC6" + catch {r XINFO STREAM _stream FULL } + assert_equal [count_log_message 0 "crashed by signal"] 0 + assert_equal [count_log_message 0 "ASSERTION FAILED"] 1 + } +} + +test {corrupt payload: fuzzer findings - stream with bad lpFirst} { + 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 _stream 0 "\x0F\x01\x10\x00\x00\x01\x7B\x0E\x52\xD2\xEC\x00\x00\x00\x00\x00\x00\x00\x00\x40\x42\x42\x00\x00\x00\x18\x00\x02\xF7\x01\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x00\x01\x02\x01\x00\x01\x00\x01\x01\x01\x00\x01\x05\x01\x03\x01\x01\x01\x00\x01\x01\x01\x82\x5F\x31\x03\x05\x01\x02\x01\x01\x01\x01\x01\x01\x01\x02\x01\x05\x01\xFF\x02\x81\x00\x00\x01\x7B\x0E\x52\xD2\xED\x01\x01\x07\x6D\x79\x67\x72\x6F\x75\x70\x81\x00\x00\x01\x7B\x0E\x52\xD2\xED\x00\x01\x00\x00\x01\x7B\x0E\x52\xD2\xED\x00\x00\x00\x00\x00\x00\x00\x00\xED\xD2\x52\x0E\x7B\x01\x00\x00\x01\x01\x05\x41\x6C\x69\x63\x65\xED\xD2\x52\x0E\x7B\x01\x00\x00\x01\x00\x00\x01\x7B\x0E\x52\xD2\xED\x00\x00\x00\x00\x00\x00\x00\x00\x09\x00\xAC\x05\xC9\x97\x5D\x45\x80\xB3"} err + assert_match "*Bad data format*" $err + r ping + } +} + +test {corrupt payload: fuzzer findings - stream listpack lpPrev valgrind issue} { + 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 _stream 0 "\x0F\x01\x10\x00\x00\x01\x7B\x0E\xAE\x66\x36\x00\x00\x00\x00\x00\x00\x00\x00\x40\x42\x42\x00\x00\x00\x18\x00\x02\x01\x01\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x00\x01\x02\x01\x00\x01\x00\x01\x01\x01\x00\x01\x1D\x01\x03\x01\x24\x01\x00\x01\x01\x69\x82\x5F\x31\x03\x05\x01\x02\x01\x33\x01\x00\x01\x01\x01\x02\x01\x05\x01\xFF\x02\x81\x00\x00\x01\x7B\x0E\xAE\x66\x69\x00\x01\x07\x6D\x79\x67\x72\x6F\x75\x70\x81\x00\x00\x01\x7B\x0E\xAE\x66\x5A\x00\x01\x00\x00\x01\x7B\x0E\xAE\x66\x5A\x00\x00\x00\x00\x00\x00\x00\x00\x94\x66\xAE\x0E\x7B\x01\x00\x00\x01\x01\x05\x41\x6C\x69\x63\x65\x83\x66\xAE\x0E\x7B\x01\x00\x00\x01\x00\x00\x01\x7B\x0E\xAE\x66\x5A\x00\x00\x00\x00\x00\x00\x00\x00\x09\x00\xD5\xD7\xA5\x5C\x63\x1C\x09\x40" + catch {r XREVRANGE _stream 1618622681 606195012389} + assert_equal [count_log_message 0 "crashed by signal"] 0 + assert_equal [count_log_message 0 "ASSERTION FAILED"] 1 + } +} + +test {corrupt payload: fuzzer findings - stream with non-integer entry id} { + 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 _streambig 0 "\x0F\x03\x10\x00\x00\x01\x7B\x13\x34\xC3\xB2\x00\x00\x00\x00\x00\x00\x00\x00\xC3\x40\x4F\x40\x5C\x18\x5C\x00\x00\x00\x24\x00\x05\x01\x00\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x40\x10\x00\x80\x20\x01\x00\x01\x20\x03\x00\x05\x20\x1C\x40\x09\x05\x01\x01\x82\x5F\x31\x03\x80\x0D\x00\x02\x20\x0D\x00\x02\xA0\x19\x00\x03\x20\x0B\x02\x82\x5F\x33\xA0\x19\x00\x04\x20\x0D\x00\x04\x20\x19\x00\xFF\x10\x00\x00\x01\x7B\x13\x34\xC3\xB2\x00\x00\x00\x00\x00\x00\x00\x05\xC3\x40\x56\x40\x61\x18\x61\x00\x00\x00\x24\x00\x05\x01\x00\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x40\x10\x00\x00\x20\x01\x06\x01\x01\x82\x5F\x35\x03\x05\x20\x1E\x40\x0B\x03\x01\x01\x06\x01\x40\x0B\x03\x01\x01\xDF\xFB\x20\x05\x02\x82\x5F\x37\x60\x1A\x20\x0E\x00\xFC\x20\x05\x00\x08\xC0\x1B\x00\xFD\x20\x0C\x02\x82\x5F\x39\x20\x1B\x00\xFF\x10\x00\x00\x01\x7B\x13\x34\xC3\xB3\x00\x00\x00\x00\x00\x00\x00\x03\xC3\x3D\x40\x4A\x18\x4A\x00\x00\x00\x15\x00\x02\x01\x00\x01\x02\x01\x84\x69\x74\x65\x6D\x05\x85\x76\x61\x6C\x75\x65\x06\x40\x10\x00\x00\x20\x01\x40\x00\x00\x05\x60\x07\x02\xDF\xFD\x02\xC0\x23\x09\x01\x01\x86\x75\x6E\x69\x71\x75\x65\x07\xA0\x2D\x02\x08\x01\xFF\x0C\x81\x00\x00\x01\x7B\x13\x34\xC3\xB4\x00\x00\x09\x00\x9D\xBD\xD5\xB9\x33\xC4\xC5\xFF"} err + #catch {r XINFO STREAM _streambig FULL } + assert_match "*Bad data format*" $err + r ping + } +} + 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