Conversation
The sequence loop in `rzb` copies each sequence's literals (`ll`) and match (`ml`) into the block buffer without checking that the output position stays within it. Out-of-bounds typed-array writes are silent no-ops in JS, so a frame whose sequences claim more output than the block can hold does not error: the loop just runs its full sequence count burning CPU. The sequence count and match lengths come from the input (up to 519935 sequences, match length up to 65539+), so a ~20-byte frame can pin a core for tens of seconds to minutes and freeze the event loop / main thread, while returning a garbage result. Node's native zstd rejects the same bytes immediately (dstSize_tooSmall). Reject a sequence whose `ll + ml` would carry the output past the block buffer before copying. For valid frames the cumulative literal+match length equals the block's regenerated size, which is at most the block buffer length, so the guard never fires on conformant input; the window-back-reference branch only redistributes the same `ml`, which the pre-copy check already covers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The sequence loop in
rzbcopies each sequence's literals (ll) and match (ml) into the block buffer without checking that the running output positionoubtstays within it. Out-of-bounds typed-array writes are silent no-ops in JS, so a frame whose sequences claim more output than the block can hold does not error — the loop just runs its full sequence count, burning CPU.The sequence count and match lengths are decoded from the input (up to 519935 sequences via the
0xFFextended encoding, match length up to 65539+), so a ~20-byte frame can pin a core for a long time.decompress()andDecompress.push()both reach this path.Impact (measured)
A ~20-byte frame, fixed except for the declared sequence count
ns:masterThe maximum declarable
ns(519935) extrapolates to minutes of 100% CPU, and the returned output is only garbage (240 bytes). This freezes the browser main thread or blocks Node's event loop from an unauthenticated payload. Node's native zstd rejects the same bytes instantly (ZSTD_error_dstSize_tooSmall).Fix
Reject a sequence whose
ll + mlwould carry the output past the block buffer, before copying. For a valid frame the cumulative literal+match length equals the block's regenerated size, which is at mostst.m= the block buffer length, sooubt + ll + mlnever exceeds it and the guard does not fire on conformant input. The window-back-reference branch only redistributes the samemlbetween window and buffer, so the pre-copy check on the originalmlcovers it.Test
Adds
tests/dos_malformed_sequences_test.ts: a 17-byte frame whose sequences claim far more output than the 240-byte block must be rejected. It fails onmaster(decompressreturns instead of throwing) and passes with the fix. Existing suite unaffected: 4 passed / 0 failed (the "1,000,000 nuls" and "Lorem ipsum" cases exercise real sequences/matches and still pass, confirming the guard never fires on valid data). After the fix the DoS inputs above throw in ~0.4 ms instead of hanging.