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 <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>

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 */

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: <type=09><rdb-string-len=18><zipmap><rdb-ver=5000><crc=0>
+
+        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}
 }