mirror of
https://github.com/redis/redis.git
synced 2026-10-28 08:25:04 +08:00
Keep XTRIM cursor valid after listpack flags re-encode (#16044)
## Problem Stream entries store a small flags integer in a listpack. A crafted DUMP/RESTORE payload can encode that flags element as a 64-bit integer instead of the 7-bit encoding that actually fits the value. Integrity checks still accept the listpack, so the key loads even with `sanitize-dump-payload yes`. `XTRIM` walks the node, marks entries deleted via `lpReplaceInteger()`, and then used to restore the scan cursor as `lp + (p - lp)`. Replacing a padded encoding with the minimal one shrinks the listpack (and may realloc it), so that saved offset no longer points at the next entry — the trim walk continues from the wrong place and can crash. ## Solution `streamTrim()` no longer saves `p` as an offset from the start of the listpack. It saves the distance from the element after the flags slot (`p - lpNext(lp, pcopy)`) and restores the cursor with `lpNext(lp, pcopy) + delta` after `lpReplaceInteger()`, so the walk continues at the next entry even if the flags rewrite reallocates or shrinks the listpack. Adds `corrupt-dump.tcl` coverage that RESTOREs the same non-minimal-flags payload and runs `XTRIM MAXLEN 1`. (cherry picked from commit c9c9dd76d47725cc088ebf1f361ab2291e270280) Co-authored-by: Sergei Georgiev <s_ggeorgiev@yahoo.com>
This commit is contained in:
1 parent
e4eb89782d
commit
788eb04400
2 files changed
+24
-3
No files matched your search
+6
-3
@@ -1176,14 +1176,17 @@ int64_t streamTrim(redisDb *db, stream *s, streamAddTrimArgs *args, streamID *ma
|
||||
}
|
||||
|
||||
if (can_delete) {
|
||||
/* Mark the entry as deleted. */
|
||||
intptr_t delta = p ? (p - lp) : 0; /* p may be NULL if this was the last entry */
|
||||
/* lpReplaceInteger() may re-encode the flags element with
|
||||
* a different size and realloc the listpack, so we save
|
||||
* the distance of 'p' from the element following the flags
|
||||
* and restore it after the replacement. */
|
||||
intptr_t delta = p ? (p - lpNext(lp, pcopy)) : 0; /* p may be NULL if this was the last entry */
|
||||
flags |= STREAM_ITEM_FLAG_DELETED;
|
||||
lp = lpReplaceInteger(lp, &pcopy, flags);
|
||||
if (max_deleted_id) *max_deleted_id = currid;
|
||||
deleted_from_lp++;
|
||||
s->length--;
|
||||
if (p) p = lp + delta;
|
||||
if (p) p = lpNext(lp, pcopy) + delta;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1195,5 +1195,23 @@ test {corrupt payload: stream consumer group with overflowing entries_read} {
|
||||
}
|
||||
}
|
||||
|
||||
# Stream DUMP (RDB_TYPE_STREAM_LISTPACKS) with 6 entries (IDs 1-0..6-0,
|
||||
# field f). The first entry's flags element is stored as a 64-bit integer
|
||||
# (LP_ENCODING_64BIT_INT) instead of the minimal 7-bit encoding. CRC is stale;
|
||||
# tests skip checksum validation.
|
||||
set ::stream_nonminimal_flags "\x0f\x01\x10\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x40\x62\x62\x00\x00\x00\x23\x00\x06\x01\x00\x01\x01\x01\x81\x66\x02\x00\x01\xf4\x02\x00\x00\x00\x00\x00\x00\x00\x09\x00\x01\x00\x01\x82\x76\x30\x03\x04\x01\x02\x01\x01\x01\x00\x01\x82\x76\x31\x03\x04\x01\x02\x01\x02\x01\x00\x01\x82\x76\x32\x03\x04\x01\x02\x01\x03\x01\x00\x01\x82\x76\x33\x03\x04\x01\x02\x01\x04\x01\x00\x01\x82\x76\x34\x03\x04\x01\x02\x01\x05\x01\x00\x01\x82\x76\x35\x03\x04\x01\xff\x06\x06\x00\x00\x09\x00\x00\x00\x00\x00\x00\x00\x00\x00"
|
||||
|
||||
test {corrupt payload: stream XTRIM with non-minimally encoded flags} {
|
||||
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
|
||||
r config set sanitize-dump-payload yes
|
||||
r debug set-skip-checksum-validation 1
|
||||
assert_equal {OK} [r restore key 0 $::stream_nonminimal_flags]
|
||||
assert_equal 6 [r xlen key]
|
||||
r xtrim key maxlen 1
|
||||
assert_equal 1 [r xlen key]
|
||||
r ping
|
||||
}
|
||||
}
|
||||
|
||||
} ;# tags
|
||||
|
||||
Reference in new issue
Block a user