Unverified Commit 95cbe879 authored by debing.sun's avatar debing.sun Committed by GitHub
Browse files

sanitize dump payload for HFE (#13278)

Add the following validations:
1. Get TTL using the lpGetIntegerValue() method instead of lpGetValue(),
Ref https://github.com/redis/redis/pull/13209#discussion_r1602569422


2. The TTL of listpackex is a number in the valid range
(0~EB_EXPIRE_TIME_MAX) and ordered.
3. The TTL fields of listpackex are ordered. 
4. The TTL of hashtable is within the valid range
(0~EB_EXPIRE_TIME_MAX).

Other:
Fix the missing of handling OBJ_ENCODING_LISTPACK_EX in
dismissHashObject().

---------
Co-authored-by: default avatarOzan Tezcan <ozantezcan@gmail.com>
parent 36c3cec6
......@@ -1247,7 +1247,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
val = p; /* Keep pointer to value */
p = lpNext(lp, p);
serverAssert(!lpGetValue(p, NULL, &expire_at));
serverAssert(p && lpGetIntegerValue(p, &expire_at));
if (hashTypeIsExpired(o, expire_at) ||
(use_pattern && !stringmatchlen(pat, sdslen(pat), (char *)str, len, 0)))
......
......@@ -669,6 +669,18 @@ unsigned char *lpGetValue(unsigned char *p, unsigned int *slen, long long *lval)
return vstr;
}
/* This is just a wrapper to lpGet() that is able to get an integer from an entry directly.
* Returns 1 and stores the integer in 'lval' if the entry is an integer.
* Returns 0 if the entry is a string. */
int lpGetIntegerValue(unsigned char *p, long long *lval) {
int64_t ele_len;
if (!lpGet(p, &ele_len, NULL)) {
*lval = ele_len;
return 1;
}
return 0;
}
/* Find pointer to the entry equal to the specified entry. Skip 'skip' entries
* between every comparison. Returns NULL when the field could not be found. */
unsigned char *lpFind(unsigned char *lp, unsigned char *p, unsigned char *s,
......
......@@ -55,6 +55,7 @@ unsigned char *lpDup(unsigned char *lp);
unsigned long lpLength(unsigned char *lp);
unsigned char *lpGet(unsigned char *p, int64_t *count, unsigned char *intbuf);
unsigned char *lpGetValue(unsigned char *p, unsigned int *slen, long long *lval);
int lpGetIntegerValue(unsigned char *p, long long *lval);
unsigned char *lpFind(unsigned char *lp, unsigned char *p, unsigned char *s, uint32_t slen, unsigned int skip);
unsigned char *lpFirst(unsigned char *lp);
unsigned char *lpLast(unsigned char *lp);
......
......@@ -492,6 +492,9 @@ void dismissHashObject(robj *o, size_t size_hint) {
dismissMemory(d->ht_table[1], DICTHT_SIZE(d->ht_size_exp[1])*sizeof(dictEntry*));
} else if (o->encoding == OBJ_ENCODING_LISTPACK) {
dismissMemory(o->ptr, lpBytes((unsigned char*)o->ptr));
} else if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
listpackEx *lpt = o->ptr;
dismissMemory(lpt->lp, lpBytes((unsigned char*)lpt->lp));
} else {
serverPanic("Unknown hash encoding type");
}
......
......@@ -1817,6 +1817,7 @@ static int _lpEntryValidation(unsigned char *p, unsigned int head_count, void *u
int tuple_len;
long count;
dict *fields;
long long last_expireat;
} *data = userdata;
if (data->fields == NULL) {
......@@ -1840,6 +1841,19 @@ static int _lpEntryValidation(unsigned char *p, unsigned int head_count, void *u
}
}
/* Validate TTL field, only for listpackex. */
if (data->count % data->tuple_len == 2) {
long long expire_at;
/* Must be an integer. */
if (!lpGetIntegerValue(p, &expire_at)) return 0;
/* Must be less than EB_EXPIRE_TIME_MAX. */
if (expire_at < 0 || (unsigned long long)expire_at > EB_EXPIRE_TIME_MAX) return 0;
/* TTL fields are ordered. If the current field has TTL, the previous field must
* also have one, and the current TTL must be greater than the previous one. */
if (expire_at != 0 && (data->last_expireat == 0 || expire_at < data->last_expireat)) return 0;
data->last_expireat = expire_at;
}
(data->count)++;
return 1;
}
......@@ -1859,7 +1873,8 @@ int lpValidateIntegrityAndDups(unsigned char *lp, size_t size, int deep, int tup
int tuple_len;
long count;
dict *fields; /* Initialisation at the first callback. */
} data = {tuple_len, 0, NULL};
long long last_expireat; /* Last field's expiry time to ensure order in TTL fields. */
} data = {tuple_len, 0, NULL, -1};
int ret = lpValidateIntegrity(lp, size, 1, _lpEntryValidation, &data);
......@@ -2257,6 +2272,11 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, redisDb* db, int rdbflags,
if (dupSearchDict != NULL) dictRelease(dupSearchDict);
return NULL;
}
if (expire > EB_EXPIRE_TIME_MAX) {
rdbReportCorruptRDB("invalid expire time: %llu", (unsigned long long)expire);
decrRefCount(o);
return NULL;
}
/* read the field name */
if ((field = rdbGenericLoadStringObject(rdb, RDB_LOAD_SDS, &fieldLen)) == NULL) {
......
......@@ -334,7 +334,7 @@ static uint64_t listpackExExpireDryRun(const robj *o) {
serverAssert(o->encoding == OBJ_ENCODING_LISTPACK_EX);
uint64_t expired = 0;
unsigned char *fptr, *s;
unsigned char *fptr;
listpackEx *lpt = o->ptr;
fptr = lpFirst(lpt->lp);
......@@ -344,10 +344,7 @@ static uint64_t listpackExExpireDryRun(const robj *o) {
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
s = lpGetValue(fptr, NULL, &val);
serverAssert(!s);
serverAssert(fptr && lpGetIntegerValue(fptr, &val));
if (!hashTypeIsExpired(o, val))
break;
......@@ -364,15 +361,14 @@ static uint64_t listpackExGetMinExpire(robj *o) {
serverAssert(o->encoding == OBJ_ENCODING_LISTPACK_EX);
long long expireAt;
unsigned char *fptr, *s;
unsigned char *fptr;
listpackEx *lpt = o->ptr;
/* As fields are ordered by expire time, first field will have the smallest
* expiry time. Third element is the expiry time of the first field */
fptr = lpSeek(lpt->lp, 2);
if (fptr != NULL) {
s = lpGetValue(fptr, NULL, &expireAt);
serverAssert(!s);
serverAssert(lpGetIntegerValue(fptr, &expireAt));
/* Check if this is a non-volatile field. */
if (expireAt != HASH_LP_NO_TTL)
......@@ -386,7 +382,7 @@ static uint64_t listpackExGetMinExpire(robj *o) {
void listpackExExpire(robj *o, ExpireInfo *info) {
serverAssert(o->encoding == OBJ_ENCODING_LISTPACK_EX);
uint64_t min = EB_EXPIRE_TIME_INVALID;
unsigned char *ptr, *field, *s;
unsigned char *ptr, *field;
listpackEx *lpt = o->ptr;
ptr = lpFirst(lpt->lp);
......@@ -397,10 +393,7 @@ void listpackExExpire(robj *o, ExpireInfo *info) {
ptr = lpNext(lpt->lp, ptr);
serverAssert(ptr);
ptr = lpNext(lpt->lp, ptr);
serverAssert(ptr);
s = lpGetValue(ptr, NULL, &val);
serverAssert(!s);
serverAssert(ptr && lpGetIntegerValue(ptr, &val));
/* Fields are ordered by expiry time. If we reached to a non-expired
* field or a non-volatile field, we know rest is not yet expired. */
......@@ -469,7 +462,7 @@ static void listpackExUpdateExpiry(robj *o, sds field,
unsigned int slen;
long long val;
unsigned char tmp[512] = {0};
unsigned char *valstr, *s, *elem;
unsigned char *valstr, *elem;
listpackEx *lpt = o->ptr;
sds tmpval = NULL;
......@@ -497,10 +490,7 @@ static void listpackExUpdateExpiry(robj *o, sds field,
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
s = lpGetValue(fptr, NULL, &currExpiry);
serverAssert(!s);
serverAssert(fptr && lpGetIntegerValue(fptr, &currExpiry));
if (currExpiry == HASH_LP_NO_TTL || (uint64_t) currExpiry >= expireAt) {
/* Found a field with no expiry time or with a higher expiry time.
......@@ -541,7 +531,7 @@ out:
/* Add new field ordered by expire time. */
void listpackExAddNew(robj *o, sds field, sds value, uint64_t expireAt) {
unsigned char *fptr, *s, *elem;
unsigned char *fptr, *elem;
listpackEx *lpt = o->ptr;
/* Shortcut, just append at the end if this is a non-volatile field. */
......@@ -557,10 +547,7 @@ void listpackExAddNew(robj *o, sds field, sds value, uint64_t expireAt) {
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
s = lpGetValue(fptr, NULL, &currExpiry);
serverAssert(!s);
serverAssert(fptr && lpGetIntegerValue(fptr, &currExpiry));
if (currExpiry == HASH_LP_NO_TTL || (uint64_t) currExpiry >= expireAt) {
/* Found a field with no expiry time or with a higher expiry time.
......@@ -594,10 +581,8 @@ SetExRes hashTypeSetExpiryListpack(HashTypeSetEx *ex, sds field,
{
long long expireTime;
uint64_t prevExpire = EB_EXPIRE_TIME_INVALID;
unsigned char *s;
s = lpGetValue(tptr, NULL, &expireTime);
serverAssert(!s);
serverAssert(lpGetIntegerValue(tptr, &expireTime));
if (expireTime != HASH_LP_NO_TTL) {
prevExpire = (uint64_t) expireTime;
......@@ -728,10 +713,7 @@ int hashTypeGetFromListpack(robj *o, sds field,
serverAssert(vptr != NULL);
h = lpNext(lpt->lp, vptr);
serverAssert(h != NULL);
h = lpGetValue(h, NULL, &expire);
serverAssert(h == NULL);
serverAssert(h && lpGetIntegerValue(h, &expire));
if (hashTypeIsExpired(o, expire))
return -1;
......@@ -1203,7 +1185,6 @@ static SetExRes hashTypeSetExListpack(redisDb *db, robj *o, sds field, HashTypeS
if (fptr != NULL) {
fptr = lpFind(lpt->lp, fptr, (unsigned char*)field, sdslen(field), 2);
if (fptr != NULL) {
unsigned char *p;
/* Grab pointer to the value (fptr points to the field) */
vptr = lpNext(lpt->lp, fptr);
serverAssert(vptr != NULL);
......@@ -1219,9 +1200,7 @@ static SetExRes hashTypeSetExListpack(redisDb *db, robj *o, sds field, HashTypeS
res = HSET_UPDATE;
}
tptr = lpNext(lpt->lp, vptr);
serverAssert(tptr != NULL);
p = lpGetValue(tptr, NULL, &expireTime);
serverAssert(!p);
serverAssert(tptr && lpGetIntegerValue(tptr, &expireTime));
if (ex) {
res = hashTypeSetExpiryListpack(ex, field, fptr, vptr, tptr,
......@@ -1410,9 +1389,7 @@ int hashTypeNext(hashTypeIterator *hi, int skipExpiredFields) {
serverAssert(vptr != NULL);
tptr = lpNext(zl, vptr);
serverAssert(tptr != NULL);
lpGetValue(tptr, NULL, &expire_time);
serverAssert(tptr && lpGetIntegerValue(tptr, &expire_time));
if (!skipExpiredFields || !hashTypeIsExpired(hi->subject, expire_time))
break;
......@@ -1662,7 +1639,7 @@ void hashTypeConvertListpackEx(robj *o, int enc, ebuckets *hexpires) {
hfieldFree(key); sdsfree(value); /* Needed for gcc ASAN */
hashTypeReleaseIterator(hi); /* Needed for gcc ASAN */
serverLogHexDump(LL_WARNING,"listpack with dup elements dump",
o->ptr,lpBytes(o->ptr));
lpt->lp,lpBytes(lpt->lp));
serverPanic("Listpack corruption detected");
}
......@@ -2820,9 +2797,7 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
fptr = lpNext(lpt->lp, fptr);
serverAssert(fptr);
lpGetValue(fptr, NULL, &expire);
serverAssert(fptr && lpGetIntegerValue(fptr, &expire));
if (expire == HASH_LP_NO_TTL) {
addReplyLongLong(c, HFE_GET_NO_TTL);
......@@ -3052,7 +3027,7 @@ void hpersistCommand(client *c) {
return;
} else if (hashObj->encoding == OBJ_ENCODING_LISTPACK_EX) {
long long prevExpire;
unsigned char *fptr, *vptr, *tptr, *s;
unsigned char *fptr, *vptr, *tptr;
listpackEx *lpt = hashObj->ptr;
addReplyArrayLen(c, numFields);
......@@ -3071,10 +3046,7 @@ void hpersistCommand(client *c) {
vptr = lpNext(lpt->lp, fptr);
serverAssert(vptr);
tptr = lpNext(lpt->lp, vptr);
serverAssert(tptr);
s = lpGetValue(tptr, NULL, &prevExpire);
serverAssert(!s);
serverAssert(tptr && lpGetIntegerValue(tptr, &prevExpire));
if (prevExpire == HASH_LP_NO_TTL) {
addReplyLongLong(c, HFE_PERSIST_NO_TTL);
......@@ -3400,7 +3372,7 @@ err_expiration:
static int hgetfReplyValueAndSetExpiry(client *c, robj *o, sds field, int flag,
uint64_t expireAt, uint64_t *minPrevExp)
{
unsigned char *fptr = NULL, *vptr = NULL, *tptr, *h;
unsigned char *fptr = NULL, *vptr = NULL, *tptr;
hfield hf = NULL;
dict *d = NULL;
dictEntry *de = NULL;
......@@ -3440,10 +3412,7 @@ static int hgetfReplyValueAndSetExpiry(client *c, robj *o, sds field, int flag,
serverAssert(vptr != NULL);
tptr = lpNext(lpt->lp, vptr);
serverAssert(tptr != NULL);
h = lpGetValue(tptr, NULL, &expire);
serverAssert(h == NULL);
serverAssert(tptr && lpGetIntegerValue(tptr, &expire));
if (expire != HASH_LP_NO_TTL)
prevExpire = expire;
......@@ -3631,7 +3600,7 @@ static int hsetfSetFieldAndReply(client *c, robj *o, sds field, sds value,
if (o->encoding == OBJ_ENCODING_LISTPACK_EX) {
long long expire;
unsigned char *fptr, *vptr = NULL, *tptr, *h;
unsigned char *fptr, *vptr = NULL, *tptr;
listpackEx *lpt = o->ptr;
fptr = lpFirst(lpt->lp);
......@@ -3640,8 +3609,7 @@ static int hsetfSetFieldAndReply(client *c, robj *o, sds field, sds value,
if (fptr != NULL) {
vptr = lpNext(lpt->lp, fptr);
tptr = lpNext(lpt->lp, vptr);
h = lpGetValue(tptr, NULL, &expire);
serverAssert(!h);
serverAssert(tptr && lpGetIntegerValue(tptr, &expire));
if (expire != HASH_LP_NO_TTL)
prevExpire = expire;
......
......@@ -19,12 +19,14 @@ proc generate_collections {suffix elements} {
# add both string values and integers
if {$j % 2 == 0} {set val $j} else {set val "_$j"}
$rd hset hash$suffix $j $val
$rd hset hashmd$suffix $j $val
$rd hexpire hashmd$suffix [expr {int(rand() * 10000)}] FIELDS 1 $j
$rd lpush list$suffix $val
$rd zadd zset$suffix $j $val
$rd sadd set$suffix $val
$rd xadd stream$suffix * item 1 value $val
}
for {set j 0} {$j < $elements * 5} {incr j} {
for {set j 0} {$j < $elements * 7} {incr j} {
$rd read ; # Discard replies
}
$rd close
......
......@@ -59,6 +59,62 @@ test {corrupt payload: valid zipped hash header, dup records} {
}
}
test {corrupt payload: hash listpackex with invalid string TTL} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload yes
catch {
r restore key 0 "\x17\x2d\x2d\x00\x00\x00\x09\x00\x81\x61\x02\x01\x01\xf4\xa6\x96\x18\xb8\x8f\x01\x00\x00\x09\x82\x66\x31\x03\x82\x76\x31\x03\x83\x66\x6f\x6f\x04\x82\x66\x32\x03\x82\x76\x32\x03\x00\x01\xff\x0c\x00\xde\x40\xe5\x37\x51\x1c\x12\x56" replace
} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: hash listpackex with TTL large than EB_EXPIRE_TIME_MAX} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload yes
catch {
r restore key 0 "\x17\x33\x33\x00\x00\x00\x09\x00\x00\x01\x00\x01\xf4\x01\xc5\x89\x95\x8f\x01\x00\x00\x09\x01\x01\x82\x5f\x31\x03\xf4\x29\x94\x97\x95\x8f\x01\x00\x00\x09\x02\x01\x02\x01\xf4\x01\x5e\xaf\x95\x8f\x01\x33\x00\x09\xff\x0c\x00\x7e\x4f\xf4\x33\xe9\xc5\x3e\x56" replace
} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: hash listpackex with unordered TTL fields} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload yes
catch {
r restore key 0 "\x17\xc3\x30\x35\x14\x35\x00\x00\x00\t\x00\x82\x66\x32\x03\x82\x76\x32\x03\xf4\x80\x73\x16\xd1\x8f\x01\x20\x12\x02\x82\x66\x31\x20\x11\x03\x31\x03\xf4\x7f\xe0\x01\x11\x00\x33\x20\x11\x04\x33\x03\x00\x01\xff\x0c\x00\xf6\x70\x29\x57\x11\x68\x9d\xe5" replace
} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: hash listpackex field without TTL should not be followed by field with TTL} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload yes
catch {
r restore key 0 "\x17\x2d\x2d\x00\x00\x00\x09\x00\x82\x66\x31\x03\x82\x76\x31\x03\x00\x01\x82\x66\x32\x03\x82\x76\x32\x03\xf4\xe0\x59\x7a\x96\x00\x00\x00\x00\x09\x82\x66\x33\x03\x82\x76\x33\x03\x00\x01\xff\x0c\x00\x42\x66\xd4\xbe\x17\xc3\x96\x72" replace
} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: hash hashtable with TTL large than EB_EXPIRE_TIME_MAX} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set hash-max-listpack-entries 0
r config set sanitize-dump-payload yes
catch {
r restore key 0 "\x16\x02\x81\x00\x01\x00\x00\x00\x00\x00\x00\x02\x66\x31\x02\x76\x31\x81\x00\x01\x00\x00\x00\x00\x00\x00\x02\x66\x32\x02\x76\x32\x0c\x00\xb9\x3c\x65\x28\x40\x94\x58\x36" replace
} err
assert_match "*Bad data format*" $err
r ping
}
}
test {corrupt payload: quicklist big ziplist prev len} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
......
......@@ -89,8 +89,8 @@ proc hrandfieldTest {activeExpireConfig} {
############################### TESTS #########################################
start_server {tags {"external:skip needs:debug"}} {
foreach type {listpack ht} {
if {$type eq "ht"} {
foreach type {listpackex hashtable} {
if {$type eq "hashtable"} {
r config set hash-max-listpack-entries 0
} else {
r config set hash-max-listpack-entries 512
......@@ -1125,6 +1125,31 @@ start_server {tags {"external:skip needs:debug"}} {
assert_equal [r hsetf myhash GETOLD DCF FVS 1 a b] ""
assert_equal [r exists myhash] 0
}
test {DUMP / RESTORE are able to serialize / unserialize a hash} {
r config set sanitize-dump-payload yes
r hmset myhash a 1 b 2 c 3
r hexpireat myhash 2524600800 fields 1 a
r hexpireat myhash 2524600801 fields 1 b
set encoded [r dump myhash]
r del myhash
r restore myhash 0 $encoded
assert_equal [lsort [r hgetall myhash]] "1 2 3 a b c"
assert_equal [r hexpiretime myhash FIELDS 3 a b c] {2524600800 2524600801 -1}
}
test {DUMP / RESTORE are able to serialize / unserialize a hash with TTL 0 for all fields} {
r config set sanitize-dump-payload yes
r hmset myhash a 1 b 2 c 3
r hexpire myhash 9999999 fields 1 a ;# make all TTLs of fields to 0
r hpersist myhash fields 1 a
assert_encoding $type myhash
set encoded [r dump myhash]
r del myhash
r restore myhash 0 $encoded
assert_equal [lsort [r hgetall myhash]] "1 2 3 a b c"
assert_equal [r hexpiretime myhash FIELDS 3 a b c] {-1 -1 -1}
}
}
r config set hash-max-listpack-entries 512
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment