From 08b0004181978fc7f111703d3613881bcd188212 Mon Sep 17 00:00:00 2001 From: Takeru Ohta Date: Sat, 25 Jul 2026 19:23:36 +0900 Subject: [PATCH 1/3] test: add failing test reproducing stack overflow reported in #88 `Read for Decoder` is defined with self-recursive calls in tail position. Rust does not guarantee tail-call elimination, so a DEFLATE stream that chains many empty stored blocks makes the recursion deep enough to overflow the thread stack. The new test builds such a stream (250_000 empty non-final stored blocks followed by a small final block) and decodes it via the gzip decoder; on master the process aborts with `has overflowed its stack`. --- src/deflate/decode.rs | 60 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/deflate/decode.rs b/src/deflate/decode.rs index 7f0f23b..f8671ef 100644 --- a/src/deflate/decode.rs +++ b/src/deflate/decode.rs @@ -218,4 +218,64 @@ mod tests { let mut decoder = Decoder::new(&input[..]); assert!(io::copy(&mut decoder, &mut io::sink()).is_err()); } + + #[test] + #[cfg(feature = "std")] + fn decode_large_deflate_stream() { + const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; + let gzip = make_large_deflate_stream(250_000); + let mut decoder = crate::gzip::Decoder::new(&gzip[..]).unwrap(); + let mut decoded = Vec::new(); + decoder.read_to_end(&mut decoded).unwrap(); + assert_eq!(decoded, WASM); + } + + /// Build a gzip stream made of DEFLATE stored blocks that decompresses + /// to the 8-byte empty WebAssembly module `\0asm\x01\x00\x00\x00`. + /// + /// The first `blocks - 1` blocks are empty non-final stored blocks and the last + /// is a final stored block carrying the wasm payload, wrapped in a gzip header/trailer. + /// + /// Each empty block adds one stack frame to libflate's recursive block decoder, + /// so large `blocks` counts produce the stack-overflow payload while decompressing + /// to identical bytes. + pub fn make_large_deflate_stream(blocks: usize) -> Vec { + /// The minimal valid WebAssembly module. + const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; + /// Gzip header. CM=deflate, OS=unknown. + const HEADER: [u8; 10] = [0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03]; + /// A non-final DEFLATE stored block of length zero: BFINAL=0, LEN=0, NLEN=0xffff. + const EMPTY_NONFINAL_STORED_BLOCK: [u8; 5] = [0x00, 0x00, 0x00, 0xff, 0xff]; + /// Compute the IEEE CRC-32 (as used by gzip) of `data`. + fn crc32(data: &[u8]) -> u32 { + let mut crc: u32 = 0xffff_ffff; + for &byte in data { + crc ^= byte as u32; + for _ in 0..8 { + let mask = (crc & 1).wrapping_neg(); + crc = (crc >> 1) ^ (0xedb8_8320 & mask); + } + } + !crc + } + + let len = WASM.len() as u16; + let mut payload = Vec::with_capacity( + HEADER.len() + (blocks - 1) * EMPTY_NONFINAL_STORED_BLOCK.len() + 21, + ); + payload.extend_from_slice(&HEADER); + for _ in 0..(blocks - 1) { + payload.extend_from_slice(&EMPTY_NONFINAL_STORED_BLOCK); + } + // Final stored block: BFINAL byte, then LEN and its ones-complement NLEN + // then the raw stored bytes. + payload.push(1); + payload.extend_from_slice(&len.to_le_bytes()); + payload.extend_from_slice(&(!len).to_le_bytes()); + payload.extend_from_slice(&WASM); + // gzip trailer: CRC32 of the uncompressed data, then ISIZE mod 2^32. + payload.extend_from_slice(&crc32(&WASM).to_le_bytes()); + payload.extend_from_slice(&(WASM.len() as u32).to_le_bytes()); + payload + } } From 9428cceb0780372928552936bc5ee9335df1ece6 Mon Sep 17 00:00:00 2001 From: Takeru Ohta Date: Sat, 25 Jul 2026 19:26:39 +0900 Subject: [PATCH 2/3] fix: iterate instead of recursing in Decoder::read (issue #88) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `deflate::Decoder::read` and `gzip::Decoder::read` were written as self-recursive tail calls that advance the stream one DEFLATE block (or one gzip member) per invocation. Rust does not guarantee tail-call elimination, so a stream carrying enough consecutive empty stored blocks — or a multi-member gzip file — can exhaust the thread stack and abort the process. Rewrite both methods as `loop`s that reuse the same stack frame, matching the pattern in the rest of the crate. Also gate the `make_large_deflate_stream` test helper on the `std` feature so `cargo test --no-default-features` still compiles. --- src/deflate/decode.rs | 34 +++++++++++++++------------------- src/gzip.rs | 21 +++++++++++---------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/deflate/decode.rs b/src/deflate/decode.rs index f8671ef..c426793 100644 --- a/src/deflate/decode.rs +++ b/src/deflate/decode.rs @@ -134,30 +134,25 @@ where R: Read, { fn read(&mut self, buf: &mut [u8]) -> io::Result { - if !self.lz77_decoder.buffer().is_empty() { - self.lz77_decoder.read(buf) - } else if self.eos { - Ok(0) - } else { + loop { + if !self.lz77_decoder.buffer().is_empty() { + return self.lz77_decoder.read(buf); + } + if self.eos { + return Ok(0); + } let bfinal = self.bit_reader.read_bit()?; let btype = self.bit_reader.read_bits(2)?; self.eos = bfinal; match btype { - 0b00 => { - self.read_non_compressed_block()?; - self.read(buf) + 0b00 => self.read_non_compressed_block()?, + 0b01 => self.read_compressed_block(&symbol::FixedHuffmanCodec)?, + 0b10 => self.read_compressed_block(&symbol::DynamicHuffmanCodec)?, + 0b11 => { + return Err(invalid_data_error!( + "btype 0x11 of DEFLATE is reserved(error) value" + )); } - 0b01 => { - self.read_compressed_block(&symbol::FixedHuffmanCodec)?; - self.read(buf) - } - 0b10 => { - self.read_compressed_block(&symbol::DynamicHuffmanCodec)?; - self.read(buf) - } - 0b11 => Err(invalid_data_error!( - "btype 0x11 of DEFLATE is reserved(error) value" - )), _ => unreachable!(), } } @@ -239,6 +234,7 @@ mod tests { /// Each empty block adds one stack frame to libflate's recursive block decoder, /// so large `blocks` counts produce the stack-overflow payload while decompressing /// to identical bytes. + #[cfg(feature = "std")] pub fn make_large_deflate_stream(blocks: usize) -> Vec { /// The minimal valid WebAssembly module. const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; diff --git a/src/gzip.rs b/src/gzip.rs index 519acfb..e2eda8f 100644 --- a/src/gzip.rs +++ b/src/gzip.rs @@ -1140,28 +1140,29 @@ where R: io::Read, { fn read(&mut self, buf: &mut [u8]) -> io::Result { - if self.eos { - return Ok(0); - } + loop { + if self.eos { + return Ok(0); + } + + let read_size = self.decoder.read(buf)?; + if read_size != 0 { + return Ok(read_size); + } - let read_size = self.decoder.read(buf)?; - if read_size == 0 { match Header::read_from(self.as_inner_mut()) { Err(e) => { if e.kind() == io::ErrorKind::UnexpectedEof { self.eos = true; - Ok(0) + return Ok(0); } else { - Err(e) + return Err(e); } } Ok(header) => { self.decoder.reset(header); - self.read(buf) } } - } else { - Ok(read_size) } } } From 7d2aaf91311738a8e1bffefe699c5c8c4b338b9e Mon Sep 17 00:00:00 2001 From: Takeru Ohta Date: Sat, 25 Jul 2026 19:51:52 +0900 Subject: [PATCH 3/3] test: polish issue #88 reproduction helper - Rename `decode_large_deflate_stream` to `test_issue_88` to match the repo's naming convention for regression tests (`test_issue_64`, `test_issues_3`). - Drop the redundant `pub` on the test-only helper. - Hoist the WASM payload constant to module scope so the test and the helper share a single definition instead of two identical copies. - Refresh the helper's docstring so it no longer implies the block decoder is still recursive. - Guard `blocks - 1` with a `debug_assert!` so misuse (`blocks == 0`) fails loudly instead of wrap-then-OOM. --- src/deflate/decode.rs | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/deflate/decode.rs b/src/deflate/decode.rs index c426793..4c89efc 100644 --- a/src/deflate/decode.rs +++ b/src/deflate/decode.rs @@ -214,10 +214,17 @@ mod tests { assert!(io::copy(&mut decoder, &mut io::sink()).is_err()); } + /// The minimal valid WebAssembly module — used as the payload of the regression + /// test below just so the decoded bytes are recognizable. + #[cfg(feature = "std")] + const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; + + // Regression test for https://github.com/sile/libflate/issues/88 : + // decoding a stream carrying many DEFLATE blocks used to blow the stack + // because `Read for Decoder` was implemented with self-recursive tail calls. #[test] #[cfg(feature = "std")] - fn decode_large_deflate_stream() { - const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; + fn test_issue_88() { let gzip = make_large_deflate_stream(250_000); let mut decoder = crate::gzip::Decoder::new(&gzip[..]).unwrap(); let mut decoded = Vec::new(); @@ -225,19 +232,18 @@ mod tests { assert_eq!(decoded, WASM); } - /// Build a gzip stream made of DEFLATE stored blocks that decompresses - /// to the 8-byte empty WebAssembly module `\0asm\x01\x00\x00\x00`. - /// - /// The first `blocks - 1` blocks are empty non-final stored blocks and the last - /// is a final stored block carrying the wasm payload, wrapped in a gzip header/trailer. - /// - /// Each empty block adds one stack frame to libflate's recursive block decoder, - /// so large `blocks` counts produce the stack-overflow payload while decompressing - /// to identical bytes. + /// Build a gzip stream that decompresses to `WASM` but is padded with + /// `blocks - 1` empty non-final DEFLATE stored blocks in front of the + /// final payload-carrying block. The empty blocks decompress to nothing, + /// so the point of a large `blocks` count is stress: each empty block + /// used to add one stack frame to `deflate::Decoder::read` and would + /// eventually overflow the thread stack (see `test_issue_88`). #[cfg(feature = "std")] - pub fn make_large_deflate_stream(blocks: usize) -> Vec { - /// The minimal valid WebAssembly module. - const WASM: [u8; 8] = [0x00, b'a', b's', b'm', 0x01, 0x00, 0x00, 0x00]; + fn make_large_deflate_stream(blocks: usize) -> Vec { + debug_assert!( + blocks >= 1, + "at least one block is required for the final payload" + ); /// Gzip header. CM=deflate, OS=unknown. const HEADER: [u8; 10] = [0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03]; /// A non-final DEFLATE stored block of length zero: BFINAL=0, LEN=0, NLEN=0xffff.