Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions lib/legacy/zstd_v01.c
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,14 @@ static size_t ZSTD_execSequence(BYTE* op,
if (seqLength > (size_t)(oend - op)) return ERROR(dstSize_tooSmall);
if (sequence.litLength > (size_t)(litLimit - *litPtr)) return ERROR(corruption_detected);
/* Now we know there are no overflow in literal nor match lengths, can use pointer checks */
if (sequence.offset > (U32)(oLitEnd - base)) return ERROR(corruption_detected);
/* Use full-precision size_t arithmetic; the previous `(U32)` cast on the
* `ptrdiff_t` truncated silently when `oLitEnd - base` exceeded
* `UINT32_MAX`, letting larger `sequence.offset` values bypass the
* check on 64-bit builds with multi-GiB destination buffers. Reject
* the pathological `oLitEnd < base` case before the unsigned cast so
* a negative difference cannot wrap to a huge positive bound. */
if (oLitEnd < base) return ERROR(corruption_detected);
if (sequence.offset > (size_t)(oLitEnd - base)) return ERROR(corruption_detected);
Comment thread
evilgensec marked this conversation as resolved.

if (endMatch > oend) return ERROR(dstSize_tooSmall); /* overwrite beyond dst buffer */
if (litEnd > litLimit) return ERROR(corruption_detected); /* overRead beyond lit buffer */
Expand All @@ -1754,9 +1761,21 @@ static size_t ZSTD_execSequence(BYTE* op,
size_t qutt = 12;
U64 saved[2];

/* check */
/* The earlier `sequence.offset > (size_t)(oLitEnd - base)` check
* (with `op == oLitEnd` at this point, since `op += litLength`
* already ran) guarantees that `op - sequence.offset >= base`,
* so the `match = op - sequence.offset` pointer formed above is
* always within the destination object and does not invoke C's
* out-of-bounds pointer-arithmetic UB. The `if (match < base)`
* post-check is kept as defense-in-depth.
*
* The previous `sequence.offset > (size_t)base` check compared
* the offset against the numeric address of the `base` pointer,
* which is meaningless: on 64-bit systems `base` is a high
* virtual address that no 32-bit offset can exceed; on 32-bit
* systems the check fires only on accidental low-address
* allocations rather than on any underflow condition. */
if (match < base) return ERROR(corruption_detected);
if (sequence.offset > (size_t)base) return ERROR(corruption_detected);

/* save beginning of literal sequence, in case of write overlap */
if (overlapRisk)
Expand Down