From fea0b4064cf612d1c365b032326832bff0946bd9 Mon Sep 17 00:00:00 2001 From: Ran Shidlansik Date: Wed, 6 May 2026 02:48:38 +0300 Subject: [PATCH] Fix invalid memory access in RESTORE with malformed zipmap (CVE-2026-25243) (#3619) Root cause: zipmapValidateIntegrity() and zipmapNext() use different methods to calculate pointer advancement for length-encoded fields. Validation reads the actual encoded size via zipmapGetEncodedLengthSize() (which returns 5 for the 0xFE prefix), but zipmapRawKeyLength() (used by zipmapNext during hash conversion) recalculates via zipmapEncodeLength() which returns 1 for decoded lengths < 254. A crafted zipmap with an overlong 5-byte encoding for a small length passes validation but causes a 4-byte pointer mismatch in zipmapNext(), leading to heap buffer over-reads during the zipmap-to-listpack conversion. Fix: add sanity checks in zipmapValidateIntegrity() to reject entries where the decoded length < ZIPMAP_BIGLEN (254) but the encoding uses more than 1 byte. This is applied to both field-name and value lengths. Test: added a regression test in tests/unit/dump.tcl that crafts a RESTORE payload with a 2-entry zipmap where the first field uses an overlong 5-byte length encoding for value 3. Post-patch, this is cleanly rejected by zipmapValidateIntegrity(). Pre-patch, the misaligned zipmapNext() reads garbage (confirmed via server log: "Hash zipmap with dup elements, or big length (0)") which also produces an error, so the test serves as a defense-in-depth regression anchor rather than a strict pass/fail differentiator. The actual heap over-read is detectable with AddressSanitizer builds. Signed-off-by: ikolomi Co-authored-by: ikolomi --- src/zipmap.c | 7 +++++++ tests/unit/dump.tcl | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/zipmap.c b/src/zipmap.c index 571b58b6e..5214eaa57 100644 --- a/src/zipmap.c +++ b/src/zipmap.c @@ -196,6 +196,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 */ @@ -209,6 +213,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/unit/dump.tcl b/tests/unit/dump.tcl index 31e103e6f..a8324e338 100644 --- a/tests/unit/dump.tcl +++ b/tests/unit/dump.tcl @@ -426,4 +426,45 @@ start_server {tags {"dump"}} { assert_match {*WRONGPASS*} $err } } {} {external:skip} + + test {RESTORE rejects zipmap with overlong field length encoding (CVE-2026-25243)} { + # Craft a RESTORE payload containing a hash-zipmap (RDB type 9) where + # the field-name length is encoded using the 5-byte format (0xfe prefix) + # even though the actual length (3) fits in a single byte. + # + # The bug: zipmapValidateIntegrity() walks the zipmap using the actual + # encoded size (5 bytes for 0xfe prefix), but zipmapNext() recalculates + # the encoding size via zipmapEncodeLength(NULL, len) which returns 1 + # for lengths < 254. This 4-byte mismatch causes zipmapNext() to read + # at wrong offsets during the hash conversion loop after validation, + # leading to invalid memory access (heap buffer over-read). + # + # Zipmap layout (2 entries, 24 bytes): + # 02 - zmlen (2 entries) + # fe 03000000 - field length = 3, overlong 5-byte encoding + # 616263 - "abc" + # 03 - value length = 3 + # 00 - free = 0 + # 646566 - "def" + # 03 - field length = 3 (normal, padding entry) + # 676869 - "ghi" + # 03 - value length = 3 + # 00 - free = 0 + # 6a6b6c - "jkl" + # ff - ZIPMAP_END + # + # Pre-patch: validation passes (overlong encoding walks in-bounds), + # then zipmapNext() misaligns by 4 bytes, interpreting 0x61 ('a') as a + # 97-byte field length → heap buffer over-read / crash. + # + # Post-patch: zipmapValidateIntegrity() rejects (l < 254 && s != 1). + # + # RESTORE payload: + + r debug set-skip-checksum-validation 1 + set payload "\x09\x18\x02\xfe\x03\x00\x00\x00\x61\x62\x63\x03\x00\x64\x65\x66\x03\x67\x68\x69\x03\x00\x6a\x6b\x6c\xff\x50\x00\x00\x00\x00\x00\x00\x00\x00\x00" + catch {r restore zipmap_test 0 $payload} err + r debug set-skip-checksum-validation 0 + assert_match {*Bad data format*} $err + } {} {needs:debug} }