Skip to content

Commit f6a4057

Browse files
committed
Fix ziplist and listpack overflows and truncations (CVE-2021-32627, CVE-2021-32628)
- fix possible heap corruption in ziplist and listpack resulting by trying to allocate more than the maximum size of 4GB. - prevent ziplist (hash and zset) from reaching size of above 1GB, will be converted to HT encoding, that's not a useful size. - prevent listpack (stream) from reaching size of above 1GB. - XADD will start a new listpack if the new record may cause the previous listpack to grow over 1GB. - XADD will respond with an error if a single stream record is over 1GB - List type (ziplist in quicklist) was truncating strings that were over 4GB, now it'll respond with an error.
1 parent 666ed7f commit f6a4057

File tree

13 files changed

+339
-49
lines changed

13 files changed

+339
-49
lines changed

Diff for: src/geo.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ void georadiusGeneric(client *c, int flags) {
635635
robj *zobj;
636636
zset *zs;
637637
int i;
638-
size_t maxelelen = 0;
638+
size_t maxelelen = 0, totelelen = 0;
639639

640640
if (returned_items) {
641641
zobj = createZsetObject();
@@ -650,13 +650,14 @@ void georadiusGeneric(client *c, int flags) {
650650
size_t elelen = sdslen(gp->member);
651651

652652
if (maxelelen < elelen) maxelelen = elelen;
653+
totelelen += elelen;
653654
znode = zslInsert(zs->zsl,score,gp->member);
654655
serverAssert(dictAdd(zs->dict,gp->member,&znode->score) == DICT_OK);
655656
gp->member = NULL;
656657
}
657658

658659
if (returned_items) {
659-
zsetConvertToZiplistIfNeeded(zobj,maxelelen);
660+
zsetConvertToZiplistIfNeeded(zobj,maxelelen,totelelen);
660661
setKey(c,c->db,storekey,zobj);
661662
decrRefCount(zobj);
662663
notifyKeyspaceEvent(NOTIFY_ZSET,"georadiusstore",storekey,

Diff for: src/listpack.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ int lpEncodeGetType(unsigned char *ele, uint32_t size, unsigned char *intenc, ui
283283
} else {
284284
if (size < 64) *enclen = 1+size;
285285
else if (size < 4096) *enclen = 2+size;
286-
else *enclen = 5+size;
286+
else *enclen = 5+(uint64_t)size;
287287
return LP_ENCODING_STRING;
288288
}
289289
}

Diff for: src/quicklist.c

+15-2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
*/
3030

3131
#include <string.h> /* for memcpy */
32+
#include "redisassert.h"
3233
#include "quicklist.h"
3334
#include "zmalloc.h"
3435
#include "ziplist.h"
@@ -43,11 +44,16 @@
4344
#define REDIS_STATIC static
4445
#endif
4546

46-
/* Optimization levels for size-based filling */
47+
/* Optimization levels for size-based filling.
48+
* Note that the largest possible limit is 16k, so even if each record takes
49+
* just one byte, it still won't overflow the 16 bit count field. */
4750
static const size_t optimization_level[] = {4096, 8192, 16384, 32768, 65536};
4851

4952
/* Maximum size in bytes of any multi-element ziplist.
50-
* Larger values will live in their own isolated ziplists. */
53+
* Larger values will live in their own isolated ziplists.
54+
* This is used only if we're limited by record count. when we're limited by
55+
* size, the maximum limit is bigger, but still safe.
56+
* 8k is a recommended / default size limit */
5157
#define SIZE_SAFETY_LIMIT 8192
5258

5359
/* Minimum ziplist size in bytes for attempting compression. */
@@ -449,6 +455,8 @@ REDIS_STATIC int _quicklistNodeAllowInsert(const quicklistNode *node,
449455
unsigned int new_sz = node->sz + sz + ziplist_overhead;
450456
if (likely(_quicklistNodeSizeMeetsOptimizationRequirement(new_sz, fill)))
451457
return 1;
458+
/* when we return 1 above we know that the limit is a size limit (which is
459+
* safe, see comments next to optimization_level and SIZE_SAFETY_LIMIT) */
452460
else if (!sizeMeetsSafetyLimit(new_sz))
453461
return 0;
454462
else if ((int)node->count < fill)
@@ -468,6 +476,8 @@ REDIS_STATIC int _quicklistNodeAllowMerge(const quicklistNode *a,
468476
unsigned int merge_sz = a->sz + b->sz - 11;
469477
if (likely(_quicklistNodeSizeMeetsOptimizationRequirement(merge_sz, fill)))
470478
return 1;
479+
/* when we return 1 above we know that the limit is a size limit (which is
480+
* safe, see comments next to optimization_level and SIZE_SAFETY_LIMIT) */
471481
else if (!sizeMeetsSafetyLimit(merge_sz))
472482
return 0;
473483
else if ((int)(a->count + b->count) <= fill)
@@ -487,6 +497,7 @@ REDIS_STATIC int _quicklistNodeAllowMerge(const quicklistNode *a,
487497
* Returns 1 if new head created. */
488498
int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) {
489499
quicklistNode *orig_head = quicklist->head;
500+
assert(sz < UINT32_MAX); /* TODO: add support for quicklist nodes that are sds encoded (not zipped) */
490501
if (likely(
491502
_quicklistNodeAllowInsert(quicklist->head, quicklist->fill, sz))) {
492503
quicklist->head->zl =
@@ -510,6 +521,7 @@ int quicklistPushHead(quicklist *quicklist, void *value, size_t sz) {
510521
* Returns 1 if new tail created. */
511522
int quicklistPushTail(quicklist *quicklist, void *value, size_t sz) {
512523
quicklistNode *orig_tail = quicklist->tail;
524+
assert(sz < UINT32_MAX); /* TODO: add support for quicklist nodes that are sds encoded (not zipped) */
513525
if (likely(
514526
_quicklistNodeAllowInsert(quicklist->tail, quicklist->fill, sz))) {
515527
quicklist->tail->zl =
@@ -852,6 +864,7 @@ REDIS_STATIC void _quicklistInsert(quicklist *quicklist, quicklistEntry *entry,
852864
int fill = quicklist->fill;
853865
quicklistNode *node = entry->node;
854866
quicklistNode *new_node = NULL;
867+
assert(sz < UINT32_MAX); /* TODO: add support for quicklist nodes that are sds encoded (not zipped) */
855868

856869
if (!node) {
857870
/* we have no reference node, so let's create only node in the list */

Diff for: src/rdb.c

+24-12
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
15611561
} else if (rdbtype == RDB_TYPE_ZSET_2 || rdbtype == RDB_TYPE_ZSET) {
15621562
/* Read list/set value. */
15631563
uint64_t zsetlen;
1564-
size_t maxelelen = 0;
1564+
size_t maxelelen = 0, totelelen = 0;
15651565
zset *zs;
15661566

15671567
if ((zsetlen = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL;
@@ -1598,15 +1598,19 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
15981598

15991599
/* Don't care about integer-encoded strings. */
16001600
if (sdslen(sdsele) > maxelelen) maxelelen = sdslen(sdsele);
1601+
totelelen += sdslen(sdsele);
16011602

16021603
znode = zslInsert(zs->zsl,score,sdsele);
16031604
dictAdd(zs->dict,sdsele,&znode->score);
16041605
}
16051606

16061607
/* Convert *after* loading, since sorted sets are not stored ordered. */
16071608
if (zsetLength(o) <= server.zset_max_ziplist_entries &&
1608-
maxelelen <= server.zset_max_ziplist_value)
1609-
zsetConvert(o,OBJ_ENCODING_ZIPLIST);
1609+
maxelelen <= server.zset_max_ziplist_value &&
1610+
ziplistSafeToAdd(NULL, totelelen))
1611+
{
1612+
zsetConvert(o,OBJ_ENCODING_ZIPLIST);
1613+
}
16101614
} else if (rdbtype == RDB_TYPE_HASH) {
16111615
uint64_t len;
16121616
int ret;
@@ -1635,21 +1639,25 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
16351639
return NULL;
16361640
}
16371641

1638-
/* Add pair to ziplist */
1639-
o->ptr = ziplistPush(o->ptr, (unsigned char*)field,
1640-
sdslen(field), ZIPLIST_TAIL);
1641-
o->ptr = ziplistPush(o->ptr, (unsigned char*)value,
1642-
sdslen(value), ZIPLIST_TAIL);
1643-
16441642
/* Convert to hash table if size threshold is exceeded */
16451643
if (sdslen(field) > server.hash_max_ziplist_value ||
1646-
sdslen(value) > server.hash_max_ziplist_value)
1644+
sdslen(value) > server.hash_max_ziplist_value ||
1645+
!ziplistSafeToAdd(o->ptr, sdslen(field)+sdslen(value)))
16471646
{
1648-
sdsfree(field);
1649-
sdsfree(value);
16501647
hashTypeConvert(o, OBJ_ENCODING_HT);
1648+
ret = dictAdd((dict*)o->ptr, field, value);
1649+
if (ret == DICT_ERR) {
1650+
rdbExitReportCorruptRDB("Duplicate hash fields detected");
1651+
}
16511652
break;
16521653
}
1654+
1655+
/* Add pair to ziplist */
1656+
o->ptr = ziplistPush(o->ptr, (unsigned char*)field,
1657+
sdslen(field), ZIPLIST_TAIL);
1658+
o->ptr = ziplistPush(o->ptr, (unsigned char*)value,
1659+
sdslen(value), ZIPLIST_TAIL);
1660+
16531661
sdsfree(field);
16541662
sdsfree(value);
16551663
}
@@ -1726,6 +1734,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) {
17261734
while ((zi = zipmapNext(zi, &fstr, &flen, &vstr, &vlen)) != NULL) {
17271735
if (flen > maxlen) maxlen = flen;
17281736
if (vlen > maxlen) maxlen = vlen;
1737+
if (!ziplistSafeToAdd(zl, (size_t)flen + vlen)) {
1738+
rdbExitReportCorruptRDB("Hash zipmap too big (%u)", flen);
1739+
}
1740+
17291741
zl = ziplistPush(zl, fstr, flen, ZIPLIST_TAIL);
17301742
zl = ziplistPush(zl, vstr, vlen, ZIPLIST_TAIL);
17311743
}

Diff for: src/server.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -1999,7 +1999,7 @@ unsigned char *zzlFirstInRange(unsigned char *zl, zrangespec *range);
19991999
unsigned char *zzlLastInRange(unsigned char *zl, zrangespec *range);
20002000
unsigned long zsetLength(const robj *zobj);
20012001
void zsetConvert(robj *zobj, int encoding);
2002-
void zsetConvertToZiplistIfNeeded(robj *zobj, size_t maxelelen);
2002+
void zsetConvertToZiplistIfNeeded(robj *zobj, size_t maxelelen, size_t totelelen);
20032003
int zsetScore(robj *zobj, sds member, double *score);
20042004
unsigned long zslGetRank(zskiplist *zsl, double score, sds o);
20052005
int zsetAdd(robj *zobj, double score, sds ele, int *flags, double *newscore);

Diff for: src/t_hash.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,22 @@
3939
* as their string length can be queried in constant time. */
4040
void hashTypeTryConversion(robj *o, robj **argv, int start, int end) {
4141
int i;
42+
size_t sum = 0;
4243

4344
if (o->encoding != OBJ_ENCODING_ZIPLIST) return;
4445

4546
for (i = start; i <= end; i++) {
46-
if (sdsEncodedObject(argv[i]) &&
47-
sdslen(argv[i]->ptr) > server.hash_max_ziplist_value)
48-
{
47+
if (!sdsEncodedObject(argv[i]))
48+
continue;
49+
size_t len = sdslen(argv[i]->ptr);
50+
if (len > server.hash_max_ziplist_value) {
4951
hashTypeConvert(o, OBJ_ENCODING_HT);
50-
break;
52+
return;
5153
}
54+
sum += len;
5255
}
56+
if (!ziplistSafeToAdd(o->ptr, sum))
57+
hashTypeConvert(o, OBJ_ENCODING_HT);
5358
}
5459

5560
/* Get the value from a ziplist encoded hash, identified by field.

Diff for: src/t_list.c

+30
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929

3030
#include "server.h"
3131

32+
#define LIST_MAX_ITEM_SIZE ((1ull<<32)-1024)
33+
3234
/*-----------------------------------------------------------------------------
3335
* List API
3436
*----------------------------------------------------------------------------*/
@@ -196,6 +198,14 @@ void listTypeConvert(robj *subject, int enc) {
196198

197199
void pushGenericCommand(client *c, int where) {
198200
int j, pushed = 0;
201+
202+
for (j = 2; j < c->argc; j++) {
203+
if (sdslen(c->argv[j]->ptr) > LIST_MAX_ITEM_SIZE) {
204+
addReplyError(c, "Element too large");
205+
return;
206+
}
207+
}
208+
199209
robj *lobj = lookupKeyWrite(c->db,c->argv[1]);
200210

201211
if (lobj && lobj->type != OBJ_LIST) {
@@ -277,6 +287,11 @@ void linsertCommand(client *c) {
277287
return;
278288
}
279289

290+
if (sdslen(c->argv[4]->ptr) > LIST_MAX_ITEM_SIZE) {
291+
addReplyError(c, "Element too large");
292+
return;
293+
}
294+
280295
if ((subject = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL ||
281296
checkType(c,subject,OBJ_LIST)) return;
282297

@@ -344,6 +359,11 @@ void lsetCommand(client *c) {
344359
long index;
345360
robj *value = c->argv[3];
346361

362+
if (sdslen(value->ptr) > LIST_MAX_ITEM_SIZE) {
363+
addReplyError(c, "Element too large");
364+
return;
365+
}
366+
347367
if ((getLongFromObjectOrReply(c, c->argv[2], &index, NULL) != C_OK))
348368
return;
349369

@@ -510,6 +530,11 @@ void lposCommand(client *c) {
510530
int direction = LIST_TAIL;
511531
long rank = 1, count = -1, maxlen = 0; /* Count -1: option not given. */
512532

533+
if (sdslen(ele->ptr) > LIST_MAX_ITEM_SIZE) {
534+
addReplyError(c, "Element too large");
535+
return;
536+
}
537+
513538
/* Parse the optional arguments. */
514539
for (int j = 3; j < c->argc; j++) {
515540
char *opt = c->argv[j]->ptr;
@@ -610,6 +635,11 @@ void lremCommand(client *c) {
610635
long toremove;
611636
long removed = 0;
612637

638+
if (sdslen(obj->ptr) > LIST_MAX_ITEM_SIZE) {
639+
addReplyError(c, "Element too large");
640+
return;
641+
}
642+
613643
if ((getLongFromObjectOrReply(c, c->argv[2], &toremove, NULL) != C_OK))
614644
return;
615645

Diff for: src/t_stream.c

+38-10
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@
4040
#define STREAM_ITEM_FLAG_DELETED (1<<0) /* Entry is deleted. Skip it. */
4141
#define STREAM_ITEM_FLAG_SAMEFIELDS (1<<1) /* Same fields as master entry. */
4242

43+
/* Don't let listpacks grow too big, even if the user config allows it.
44+
* doing so can lead to an overflow (trying to store more than 32bit length
45+
* into the listpack header), or actually an assertion since lpInsert
46+
* will return NULL. */
47+
#define STREAM_LISTPACK_MAX_SIZE (1<<30)
48+
4349
void streamFreeCG(streamCG *cg);
4450
void streamFreeNACK(streamNACK *na);
4551
size_t streamReplyWithRangeFromConsumerPEL(client *c, stream *s, streamID *start, streamID *end, size_t count, streamConsumer *consumer);
@@ -191,8 +197,11 @@ int streamCompareID(streamID *a, streamID *b) {
191197
*
192198
* The function returns C_OK if the item was added, this is always true
193199
* if the ID was generated by the function. However the function may return
194-
* C_ERR if an ID was given via 'use_id', but adding it failed since the
195-
* current top ID is greater or equal. */
200+
* C_ERR in several cases:
201+
* 1. If an ID was given via 'use_id', but adding it failed since the
202+
* current top ID is greater or equal. errno will be set to EDOM.
203+
* 2. If a size of a single element or the sum of the elements is too big to
204+
* be stored into the stream. errno will be set to ERANGE. */
196205
int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_id, streamID *use_id) {
197206

198207
/* Generate the new entry ID. */
@@ -206,7 +215,23 @@ int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_
206215
* or return an error. Automatically generated IDs might
207216
* overflow (and wrap-around) when incrementing the sequence
208217
part. */
209-
if (streamCompareID(&id,&s->last_id) <= 0) return C_ERR;
218+
if (streamCompareID(&id,&s->last_id) <= 0) {
219+
errno = EDOM;
220+
return C_ERR;
221+
}
222+
223+
/* Avoid overflow when trying to add an element to the stream (listpack
224+
* can only host up to 32bit length sttrings, and also a total listpack size
225+
* can't be bigger than 32bit length. */
226+
size_t totelelen = 0;
227+
for (int64_t i = 0; i < numfields*2; i++) {
228+
sds ele = argv[i]->ptr;
229+
totelelen += sdslen(ele);
230+
}
231+
if (totelelen > STREAM_LISTPACK_MAX_SIZE) {
232+
errno = ERANGE;
233+
return C_ERR;
234+
}
210235

211236
/* Add the new entry. */
212237
raxIterator ri;
@@ -265,9 +290,10 @@ int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_
265290
* if we need to switch to the next one. 'lp' will be set to NULL if
266291
* the current node is full. */
267292
if (lp != NULL) {
268-
if (server.stream_node_max_bytes &&
269-
lp_bytes >= server.stream_node_max_bytes)
270-
{
293+
size_t node_max_bytes = server.stream_node_max_bytes;
294+
if (node_max_bytes == 0 || node_max_bytes > STREAM_LISTPACK_MAX_SIZE)
295+
node_max_bytes = STREAM_LISTPACK_MAX_SIZE;
296+
if (lp_bytes + totelelen >= node_max_bytes) {
271297
lp = NULL;
272298
} else if (server.stream_node_max_entries) {
273299
int64_t count = lpGetInteger(lpFirst(lp));
@@ -1267,11 +1293,13 @@ void xaddCommand(client *c) {
12671293

12681294
/* Append using the low level function and return the ID. */
12691295
if (streamAppendItem(s,c->argv+field_pos,(c->argc-field_pos)/2,
1270-
&id, id_given ? &id : NULL)
1271-
== C_ERR)
1296+
&id, id_given ? &id : NULL) == C_ERR)
12721297
{
1273-
addReplyError(c,"The ID specified in XADD is equal or smaller than the "
1274-
"target stream top item");
1298+
if (errno == EDOM)
1299+
addReplyError(c,"The ID specified in XADD is equal or smaller than "
1300+
"the target stream top item");
1301+
else
1302+
addReplyError(c,"Elements are too large to be stored");
12751303
return;
12761304
}
12771305
addReplyStreamID(c,&id);

0 commit comments

Comments
 (0)