Skip to content

fix: two arithmetic overflows in the implode decoder - #1028

Merged
Pr0methean merged 2 commits into
zip-rs:masterfrom
yuxi-liu-wired:fix/implode-overflow
Oct 9, 2026
Merged

Pr0methean merged 2 commits into
zip-rs:masterfrom
yuxi-liu-wired:fix/implode-overflow

Conversation

@yuxi-liu-wired

Copy link
Copy Markdown
Contributor

Problem

Two integer overflows in the legacy-zip implode decoder (src/legacy/). Both panic in debug
builds and wrap in release builds, where the read then fails with an error. The PR template asks
for all methods to stay panic-free.

  1. read_huffman_code (implode.rs) computes the number of code-length bytes as
    (byte + 1) as usize with byte: u8. A header byte of 0xff means 256 bytes, which is valid
    (the literal table has 256 symbols), and 0xff + 1 overflows u8:

    panicked at src/legacy/implode.rs:29:21: attempt to add with overflow
    

    hwzip, which this decoder is ported from, widens first: num_bytes = (size_t)byte + 1.

  2. HuffmanDecoder::huffman_decode (huffman.rs) computes the bits left as
    length - is.position_in_bits(). The bit reader can already be past the end of the data
    when a symbol is decoded, so the subtraction underflows:

    panicked at src/legacy/huffman.rs:141:67: attempt to subtract with overflow
    

    Reached through ZipArchive and through the streaming reader
    (read_zipfile_from_stream) on a truncated or crafted imploded entry.

Change

  • num_bytes = usize::from(byte) + 1.
  • In huffman_decode, compute the remaining bits with saturating_sub; when none are left,
    return an UnexpectedEof error (the stream ended in the middle of a symbol). The second
    lookup is clamped the same way.

Test

In src/legacy/implode.rs, next to the existing Hamlet test:

  • code_length_table_with_256_bytes: a 256-entry code-length table (header byte 0xff) must
    decode. Panics on master.
  • truncated_stream_is_an_error: the Hamlet implode stream truncated at every length must
    return an error or a prefix of the expected output, never panic. Panics on master at 93 of 249
    bytes.

The existing legacy tests pass unchanged. The suite passes on 1.88, stable and nightly with
default features, --no-default-features, --all-features, and the two single-deflate feature
sets; cargo clippy --all-targets -- -D warnings (default, --no-default-features,
--features legacy-zip, --all-features) and cargo doc --no-deps are clean; cargo fmt --check --all is clean.

Found with a fuzzer that reads generated archives through the streaming reader.

Both panic in debug builds (overflow checks) on crafted entries and wrap in
release builds, where the read then fails with an error.

read_huffman_code computed the number of code-length bytes as
`(byte + 1) as usize` with `byte: u8`. A header byte of 0xff means 256 bytes,
which is valid (the literal table has 256 symbols), and overflows the u8
addition. hwzip, which this decoder is ported from, widens first:
`num_bytes = (size_t)byte + 1`.

HuffmanDecoder::huffman_decode computed the bits left as
`length - is.position_in_bits()`. The bit reader can already be past the end
of the data when a symbol is decoded (the previous symbol's length is only
known after the lookup, and hwzip zero-pads past the end), so the subtraction
underflows. Treat no bits left as the end of the stream: return
UnexpectedEof, and clamp the second lookup the same way.

Tests: a 256-entry code-length table must decode; the Hamlet implode stream
truncated at every length must give an error or a prefix of the output, never
a panic. Both fail on master.
@Pr0methean Pr0methean added this to the 9.0.1 milestone Oct 8, 2026
@Pr0methean
Pr0methean added this pull request to the merge queue Oct 9, 2026
Merged via the queue into zip-rs:master with commit 2528ff7 Oct 9, 2026
134 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants