From c8b4b81a9c669a383ecf7096c72401dea7b94b49 Mon Sep 17 00:00:00 2001 From: Sergei Georgiev Date: Thu, 27 Nov 2025 11:41:11 +0200 Subject: [PATCH] Invalid Memory Access in Redis RESTORE Command (CVE-2026-25243) --- src/rdb.c | 6 +-- src/sds.c | 9 +++- src/zipmap.c | 7 +++ tests/integration/corrupt-dump.tcl | 82 ++++++++++++++++++++++++++++++ tests/unit/dump.tcl | 15 ++++++ 5 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index ed30b6523ec..73da8b6f15d 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2284,11 +2284,12 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { /* search for duplicate records */ sds field = sdstrynewlen(fstr, flen); - if (!field || dictAdd(dupSearchDict, field, NULL) != DICT_OK || - !lpSafeToAdd(lp, (size_t)flen + vlen)) { + if (!field || !lpSafeToAdd(lp, (size_t)flen + vlen) || + dictAdd(dupSearchDict, field, NULL) != DICT_OK) { rdbReportCorruptRDB("Hash zipmap with dup elements, or big length (%u)", flen); dictRelease(dupSearchDict); sdsfree(field); + lpFree(lp); zfree(encoded); o->ptr = NULL; decrRefCount(o); @@ -2757,7 +2758,6 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { " loading a stream consumer " "group"); decrRefCount(o); - streamFreeNACK(nack); return NULL; } } diff --git a/src/sds.c b/src/sds.c index 2cc5b231681..ad36beb4867 100644 --- a/src/sds.c +++ b/src/sds.c @@ -112,7 +112,14 @@ sds _sdsnewlen(const void *init, size_t initlen, int trymalloc) { unsigned char *fp; /* flags pointer. */ size_t usable; - assert(initlen + hdrlen + 1 > initlen); /* Catch size_t overflow */ + if (trymalloc) { + /* protect against size_t overflow */ + if (initlen + hdrlen + 1 <= initlen) + return NULL; + } else { + assert(initlen + hdrlen + 1 > initlen); /* Catch size_t overflow */ + } + sh = trymalloc? s_trymalloc_usable(hdrlen+initlen+1, &usable) : s_malloc_usable(hdrlen+initlen+1, &usable); diff --git a/src/zipmap.c b/src/zipmap.c index 4e984ba6d9d..4fb06250f19 100644 --- a/src/zipmap.c +++ b/src/zipmap.c @@ -405,6 +405,10 @@ int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) { /* read the field name length */ l = zipmapDecodeLength(p); + /* Sanity check: length < 254 must be encoded in 1 byte, not 5 bytes */ + if (l < ZIPMAP_BIGLEN && s != 1) + return 0; + p += s; /* skip the encoded field size */ p += l; /* skip the field */ @@ -420,6 +424,9 @@ int zipmapValidateIntegrity(unsigned char *zm, size_t size, int deep) { /* read the value length */ l = zipmapDecodeLength(p); + /* Sanity check: length < 254 must be encoded in 1 byte, not 5 bytes */ + if (l < ZIPMAP_BIGLEN && s != 1) + return 0; p += s; /* skip the encoded value size*/ e = *p++; /* skip the encoded free space (always encoded in one byte) */ p += l+e; /* skip the value and free space */ diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index 3c9e5ce811f..84785f3985e 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -829,5 +829,87 @@ test {corrupt payload: fuzzer findings - set with duplicate elements causes sdif } } {} {logreqres:skip} ;# This test violates {"uniqueItems": true} +test {corrupt payload: zipmap - element wouldn't fit in listpack} { + # Redis converts legacy zipmap encoded hashes to listpacks. + # This test creates a zipmap entry with a 1GB value which cannot + # fit into a listpack and verifies that RESTORE fails. + + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no proto-max-bulk-len 2147483648 client-query-buffer-limit 2147483648]] { + proc zipmap_encode_len {len} { + if {$len < 254} { + return [binary format c $len] + } else { + return [binary format ci 254 $len] + } + } + r config set sanitize-dump-payload no + + # Generates Zipmap with 1GB value - should fail lpSafeToAdd check + set val_len [expr {1024 * 1024 * 1024 + 1}] + + # Zipmap has 1 element + set zm [binary format c 1] + # Field is 1 byte long + append zm [zipmap_encode_len 1] + append zm "k" + # Value is 1GB long + append zm [zipmap_encode_len $val_len] + append zm [binary format c 0] + append zm [string repeat "A" $val_len] + # ZIPMAP_END marker + append zm [binary format c 255] + # Prepend RDB header + set zm_len [string length $zm] + set rdb_len [binary format cI 0x80 $zm_len] + set dump [binary format c 9] + append dump $rdb_len + append dump $zm + append dump [binary format s 9] + append dump [binary format w 0] + + catch {r RESTORE _hash 0 $dump} err + assert_match "*Bad data format*" $err + } +} {} {large-memory} + +test {corrupt payload: zipmap - 5 bytes length encoding for a small field} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no]] { + catch { + r restore key 0 "\x09\x11\x01\xfe\x04\x00\x00\x00\x01\x00\xff\x00\x04\x00\x76\x61\x6c\x31\xff\x09\x00\xf9\xd5\xa4\xf7\x7d\x00\x3f\x1b" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*integrity check failed*" 0 + } +} + +test {corrupt payload: zipmap - 5 bytes length encoding for a small value} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no]] { + catch { + r restore key 0 "\x09\x0e\x01\x01\x6b\xfe\x04\x00\x00\x00\x00\x76\x61\x6c\x31\xff\x09\x00\xd0\xf9\xe4\x1d\xe4\xfb\x11\x4c" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*integrity check failed*" 0 + } +} + +test {corrupt payload: zipmap - 5 bytes length encoding and a huge field} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r restore key 0 "\x09\x41\x15\x02\x04\x6b\x65\x79\x31\x04\x00\x76\x61\x6c\x31\xfe\x04\x00\x00\x00\xfe\xff\xff\xff\xfd\x00\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\x42\xff\x09\x00\x54\x2f\x0a\xca\x4e\x5c\x49\x9f" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*integrity check failed*" 0 + } +} + +test {corrupt payload: stream - duplicated consumer PEL entry} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + catch { + r restore key 0 "\x15\x01\x10\x00\x00\x01\x9b\x0d\x56\xa9\xb7\x00\x00\x00\x00\x00\x00\x00\x00\xc3\x39\x40\x42\x15\x42\x00\x00\x00\x11\x00\x02\x01\x00\x01\x01\x01\x86\x66\x69\x65\x6c\x64\x31\x07\x00\x01\x40\x0f\x0a\x00\x01\x86\x76\x61\x6c\x75\x65\x31\x07\x04\x20\x0b\x02\xcd\xd9\x02\xe0\x01\x22\x01\x32\x07\x80\x1a\x04\x32\x07\x06\x01\xff\x02\x81\x00\x00\x01\x9b\x0d\x56\xb7\x90\x00\x81\x00\x00\x01\x9b\x0d\x56\xa9\xb7\x00\x00\x00\x02\x01\x07\x6d\x79\x67\x72\x6f\x75\x70\x81\x00\x00\x01\x9b\x0d\x56\xb7\x90\x00\x02\x02\x00\x00\x01\x9b\x0d\x56\xa9\xb7\x00\x00\x00\x00\x00\x00\x00\x00\x80\xd9\x56\x0d\x9b\x01\x00\x00\x01\x00\x00\x01\x9b\x0d\x56\xb7\x90\x00\x00\x00\x00\x00\x00\x00\x00\x80\xd9\x56\x0d\x9b\x01\x00\x00\x01\x01\x09\x63\x6f\x6e\x73\x75\x6d\x65\x72\x31\x80\xd9\x56\x0d\x9b\x01\x00\x00\x80\xd9\x56\x0d\x9b\x01\x00\x00\x02\x00\x00\x01\x9b\x0d\x56\xa9\xb7\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x9b\x0d\x56\xa9\xb7\x00\x00\x00\x00\x00\x00\x00\x00\x0c\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09\x00\x4b\xe0\x99\x30\x67\x4d\xe5\x87" + } err + assert_match "*Bad data format*" $err + verify_log_message 0 "*Duplicated consumer PEL entry*" 0 + } +} } ;# tags diff --git a/tests/unit/dump.tcl b/tests/unit/dump.tcl index dd759529038..c96da0e1d7a 100644 --- a/tests/unit/dump.tcl +++ b/tests/unit/dump.tcl @@ -124,6 +124,21 @@ start_server {tags {"dump"}} { close_replication_stream $repl } {} {needs:repl} + test {RESTORE fail with invalid payload size} { + r debug set-skip-checksum-validation 1 + # Payload with mismatched size: claims 0xFFFFFFFFFFFFFFF7 bytes (max uint64 - 8) but provides no data + # \x00 = String type + # \x81 = 64-bit length marker + # \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xF7 = 18446744073709551607 in big-endian + # \x0c\x00 = RDB version + # \x00... = fake CRC64 + set encoded "\x00\x81\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xF7\x09\x00\x00\x00\x00\x00\x00\x00\x00\x00" + r del test + catch {r restore test 0 $encoded} e + r debug set-skip-checksum-validation 0 + set e + } {*Bad data format*} {needs:debug} + test {DUMP of non existing key returns nil} { r dump nonexisting_key } {}