FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(deflate,deflate64,zlib): report StreamEnd instead of stalling on trailing bytes (DoS) by MagicalTux · Pull Request #123 · KarpelesLab/compcol · GitHub

fix(deflate,deflate64,zlib): report StreamEnd instead of stalling on trailing bytes (DoS) - #123

Merged
MagicalTux merged 1 commit into
masterfrom
fix/deflate-trailing-bytes-stall
Aug 16, 2026
Merged

fix(deflate,deflate64,zlib): report StreamEnd instead of stalling on trailing bytes (DoS)#123
MagicalTux merged 1 commit into
masterfrom
fix/deflate-trailing-bytes-stall

Conversation

Copy link
Copy Markdown
Member

Report

A crafted stream — a valid DEFLATE/zlib payload followed by trailing garbage — makes any caller that loops until Status::StreamEnd spin forever: CPU-bound, no allocation, no output. Because RSS stays flat, neither an output-size cap nor a memory cap stops it; only a wall-clock timeout. Reachable from any consumer inflating untrusted zlib (PDF /FlateDecode, PNG IDAT). Reported against 0.6.8; reproduced identically on master.

compcol::vec::decompress_to_vec::<Deflate>(&payload[7..])  // never returns
compcol::vec::decompress_to_vec::<Zlib>(&payload[5..])     // never returns

Root cause

deflate and deflate64's raw_decode hardcoded done: false, and zlib's DecPhase::Done arm did the same. Once the BFINAL block was consumed, the state machine sat in Done returning consumed=0/written=0, and the RawDecoder→Decoder bridge mapped "input left, output not full" to Status::OutputFull — call me again — with nothing left to make progress on. vec::decompress_to_vec's OutputFull => continue then looped on unchanged state.

gzip was already immune: its Done arm swallows trailing input with a comment saying "rather than spinning on Status::OutputFull". deflate/deflate64/zlib never got the same treatment.

Fix

  1. Root cause — report done from the actual terminal state so the bridge yields StreamEnd. zlib also reports it from the no-progress guard, since the trailer can complete out of trailer_carryover without consuming caller input.
  2. Defense in depth — a progress assertion in vec::decompress_to_vec{_with,_capped,_capped_with}: an OutputFull that consumed no input and wrote no output hands off to finish() instead of looping. This bounds the helper for every codec, not just the three fixed here, which is what the reporter asked for.

Correctness

The reporter's payload is kept as a fixture. It decodes to 24 bytes and zlib's adler32 validates, so it is a legitimate 24-byte stream with trailing garbage — Ok(24) is the correct outcome, not merely a terminating one. Both framings now return those same 24 bytes instantly.

Regression tests assert the streaming-level invariant (a completed stream reports StreamEnd, and never asks to be called again without progress) so a regression fails fast rather than hanging CI.

cargo test --all-features passes; fmt and clippy -D warnings clean.

Reported-by: oxideav-pdf structure-aware fuzzing

…trailing bytes

A crafted stream — a valid DEFLATE/zlib payload followed by trailing
garbage — made any caller that loops until Status::StreamEnd spin forever:
CPU-bound, no allocation, no output. Because RSS stays flat, neither an
output-size cap nor a memory cap could stop it; only a wall-clock timeout.
Reachable from any consumer inflating untrusted zlib (PDF /FlateDecode,
PNG IDAT).

Root cause: the deflate and deflate64 `raw_decode` hardcoded `done: false`,
and zlib's `DecPhase::Done` arm did the same. Once the BFINAL block was
consumed the state machine sat in `Done` returning consumed=0/written=0,
and the RawDecoder->Decoder bridge mapped "input left, output not full" to
Status::OutputFull — i.e. "call me again" — with nothing to make progress
on. `vec::decompress_to_vec`'s `OutputFull => continue` then looped on
unchanged state.

gzip was already immune: its Done arm swallows trailing input specifically
to avoid this, but deflate/deflate64/zlib never got the same treatment.

Fixes:
- Report `done` from the actual terminal state so the bridge yields
  StreamEnd. zlib also reports it from the no-progress guard, since the
  trailer can complete out of `trailer_carryover` without consuming input.
- Add a progress assertion in `vec::decompress_to_vec{_with,_capped,
  _capped_with}`: an OutputFull that consumed no input and wrote no output
  hands off to `finish()` rather than looping. This bounds the helper for
  every codec, not just the three fixed here.

The reporter's payload is kept as a fixture; it decodes to 24 bytes whose
adler32 validates, so returning Ok is the correct outcome, not just a
terminating one.

Reported-by: oxideav-pdf structure-aware fuzzing
MagicalTux merged commit 0b7f35c into master Aug 16, 2026
43 checks passed
MagicalTux deleted the fix/deflate-trailing-bytes-stall branch August 16, 2026 20:34
MagicalTux added a commit that referenced this pull request Aug 16, 2026
…ling

Follow-up to #123. That fix covered deflate/deflate64/zlib; reviewing all
53 `raw_decode` implementations found the same defect in seven more.

A decoder parked in its terminal state that returns `done: false` with
nothing consumed and nothing written, while the caller still holds input,
is mapped by the RawDecoder->Decoder bridge to `Status::OutputFull` — "call
me again" — with nothing to progress on. Any loop waiting for StreamEnd
spins: CPU-bound, no allocation, so no output or memory cap catches it.

Fixed, each verified by reading its state machine and confirmed with a
probe that drives the real encoder output plus trailing bytes:

- zstd, xz          Done arms returned false; both are terminal (neither
                    supports concatenated frames/streams).
- lz4 block, lzo    Done after the zero-length terminator block.
- lz4 frame         only reachable by calling past StreamEnd, but the state
                    is terminal either way.
- lzx, amiga_lzx    Done arms returned false.
- rar5              returns on `State::Done` *before* the block that accepts
                    caller bytes, so a container passing the next header
                    after the payload could never make progress.
- gzip              subtler: the `BetweenMembers` arm sets `phase = Done`
                    intending to reach the Done arm that swallows trailing
                    bytes, but match arms do not fall through, so the loop's
                    no-progress check returned first and the swallow never
                    ran. Needs an explicit `continue`, like the 0x1F path.

Two call shapes matter and only the first was covered before: trailing bytes
in the same call, and trailing bytes arriving *after* the payload completes.
The second is what a container does when it hands over the next header, and
it is what xz and gzip failed — both looked safe under the first shape.

Codecs that only ever report `InputEmpty` and produce output on `finish`
(brotli, lzma, and the other buffering decoders) are correct as-is: the
trait permits that, and it terminates. The invariant asserted is not "must
report StreamEnd" but "must never report OutputFull with zero progress".

A sweep over all 50 round-trippable algorithms now reports no stalls.
tests/terminal_state.rs covers each fixed codec plus controls; rar5 is
decoder-only so it gets equivalent tests against its own fixture.
MagicalTux added a commit that referenced this pull request Aug 16, 2026
…ling (#124)

Follow-up to #123. That fix covered deflate/deflate64/zlib; reviewing all
53 `raw_decode` implementations found the same defect in seven more.

A decoder parked in its terminal state that returns `done: false` with
nothing consumed and nothing written, while the caller still holds input,
is mapped by the RawDecoder->Decoder bridge to `Status::OutputFull` — "call
me again" — with nothing to progress on. Any loop waiting for StreamEnd
spins: CPU-bound, no allocation, so no output or memory cap catches it.

Fixed, each verified by reading its state machine and confirmed with a
probe that drives the real encoder output plus trailing bytes:

- zstd, xz          Done arms returned false; both are terminal (neither
                    supports concatenated frames/streams).
- lz4 block, lzo    Done after the zero-length terminator block.
- lz4 frame         only reachable by calling past StreamEnd, but the state
                    is terminal either way.
- lzx, amiga_lzx    Done arms returned false.
- rar5              returns on `State::Done` *before* the block that accepts
                    caller bytes, so a container passing the next header
                    after the payload could never make progress.
- gzip              subtler: the `BetweenMembers` arm sets `phase = Done`
                    intending to reach the Done arm that swallows trailing
                    bytes, but match arms do not fall through, so the loop's
                    no-progress check returned first and the swallow never
                    ran. Needs an explicit `continue`, like the 0x1F path.

Two call shapes matter and only the first was covered before: trailing bytes
in the same call, and trailing bytes arriving *after* the payload completes.
The second is what a container does when it hands over the next header, and
it is what xz and gzip failed — both looked safe under the first shape.

Codecs that only ever report `InputEmpty` and produce output on `finish`
(brotli, lzma, and the other buffering decoders) are correct as-is: the
trait permits that, and it terminates. The invariant asserted is not "must
report StreamEnd" but "must never report OutputFull with zero progress".

A sweep over all 50 round-trippable algorithms now reports no stalls.
tests/terminal_state.rs covers each fixed codec plus controls; rar5 is
decoder-only so it gets equivalent tests against its own fixture.
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL