| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…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
…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.
…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.
| Back | FazBrowse Home | New Git URL |
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.
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
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