diff --git a/src/arsenic/pipeline.rs b/src/arsenic/pipeline.rs index a945489..f782209 100644 --- a/src/arsenic/pipeline.rs +++ b/src/arsenic/pipeline.rs @@ -13,6 +13,12 @@ use crate::error::Error; /// Hard cap on a single block (FORMAT-SPEC §3: blockbits ≤ 24 → 16 MiB). const MAX_BLOCK_BITS: u32 = 24; +/// Hard cap on total one-shot decode output. The block loop (`while end_flag +/// == 0`) is unbounded and the final-RLE layer can expand ~51× per block, so a +/// small crafted stream could otherwise drive `out` to unbounded size. Matches +/// the sibling sit13 decoder's `DEFAULT_OUTPUT_CAP`. +const DEFAULT_OUTPUT_CAP: usize = 256 * 1024 * 1024; + /// Outcome of a full-stream decode attempt. pub(crate) enum DecodeOutcome { /// The stream was decoded to completion (CRC verified). @@ -299,6 +305,12 @@ pub(crate) fn decode_stream(data: &[u8]) -> Result { // The final-RLE layer emits one logical output byte per loop turn, // consuming `numbytes` inverse-BWT bytes total. while byte_count < numbytes as u64 || rle_repeat > 0 { + // Bound total output across all blocks. The final-RLE layer can + // expand a 16 MiB block ~51×, so the check must live inside the + // emit loop, not merely per-block. + if out.len() >= DEFAULT_OUTPUT_CAP { + return Err(Error::Corrupt); + } if rle_repeat > 0 { out.push(rle_last); crc = crc32_update(crc, rle_last); diff --git a/src/bzip2/decoder.rs b/src/bzip2/decoder.rs index 262eeec..7ea98c9 100644 --- a/src/bzip2/decoder.rs +++ b/src/bzip2/decoder.rs @@ -296,6 +296,20 @@ impl Decoder { // Now decode symbols 50 at a time, switching tables per group // per selector. Stop when EOB (= alpha_size - 1) is seen. + // + // Anti-bomb bound: a single bzip2 block decodes to at most the + // declared block size = level * 100_000 bytes (level 1..=9). The + // RLE-2 stream we are reconstructing here (`mtf_indices`) is the + // pre-BWT/pre-MTF symbol stream and must not exceed that. We keep + // a tiny constant of slack (1024) aligned with the original + // per-run headroom, but the bound stays within a couple KB of the + // real block size — NOT a multiple of it. Without a *cumulative* + // cap a malicious stream can flush a fresh ~8 MB zero-run after + // every one of ~900_100 non-zero symbols and inflate this + // intermediate buffer to hundreds of GB before any output is + // produced (so LimitedDecoder, which only sees output bytes, can't + // stop it). Mirrors arsenic's `block.len() + count > block_size`. + let max_block_bytes: u64 = self.level as u64 * 100_000 + 1024; let eob = (alpha_size - 1) as u16; let mut mtf_indices: Vec = Vec::new(); // RLE-2 accumulator: each time we see RUNA/RUNB we extend the @@ -332,9 +346,12 @@ impl Decoder { let contrib = if s == 0 { 1 } else { 2 }; zero_run = zero_run.saturating_add(contrib * zero_weight); zero_weight = zero_weight.saturating_mul(2); - if zero_run as usize > 900_000 * 9 + 1024 { - // Anti-bomb: a zero run that exceeds the maximum - // possible block content is malformed. + // Anti-bomb (cumulative): the already-materialised indices + // plus the in-flight zero-run must not exceed the declared + // block size. This catches both a single oversized run and + // the death-by-a-thousand-runs attack where each non-zero + // symbol flushes and resets a fresh run. + if mtf_indices.len() as u64 + zero_run as u64 > max_block_bytes { return Err(Error::Corrupt); } } else { @@ -348,10 +365,18 @@ impl Decoder { if idx >= num_used { return Err(Error::Corrupt); } + // Anti-bomb (cumulative): bound the literal pushes too. + if mtf_indices.len() as u64 + 1 > max_block_bytes { + return Err(Error::Corrupt); + } mtf_indices.push(idx as u8); } } if zero_run > 0 { + // Anti-bomb (cumulative): final flush must also stay in bounds. + if mtf_indices.len() as u64 + zero_run as u64 > max_block_bytes { + return Err(Error::Corrupt); + } mtf_indices.extend(core::iter::repeat_n(0u8, zero_run as usize)); } diff --git a/src/gzip/mod.rs b/src/gzip/mod.rs index e5376fd..d28ffb9 100644 --- a/src/gzip/mod.rs +++ b/src/gzip/mod.rs @@ -281,6 +281,13 @@ impl RawDecoder for Decoder { } if self.aux_remaining == 0 { self.phase = self.next_after(DecPhase::ExtraData); + // ExtraLen leaves `aux_idx == 2`; HeaderCrc reuses the + // same counter, so reset it on the transition into the + // CRC-skip phase or the loop there would consume 0 bytes + // and feed the FHCRC field into the deflate decoder. + if self.phase == DecPhase::HeaderCrc { + self.aux_idx = 0; + } } else { return Ok(RawProgress { consumed, @@ -301,6 +308,10 @@ impl RawDecoder for Decoder { } if found_nul { self.phase = self.next_after(DecPhase::Name); + // See ExtraData arm: HeaderCrc shares `aux_idx`. + if self.phase == DecPhase::HeaderCrc { + self.aux_idx = 0; + } } else { return Ok(RawProgress { consumed, @@ -321,6 +332,10 @@ impl RawDecoder for Decoder { } if found_nul { self.phase = self.next_after(DecPhase::Comment); + // See ExtraData arm: HeaderCrc shares `aux_idx`. + if self.phase == DecPhase::HeaderCrc { + self.aux_idx = 0; + } } else { return Ok(RawProgress { consumed, diff --git a/src/lz5/mod.rs b/src/lz5/mod.rs index 7684c8a..1a21eab 100644 --- a/src/lz5/mod.rs +++ b/src/lz5/mod.rs @@ -612,14 +612,20 @@ impl RawDecoder for Decoder { return Err(Error::Corrupt); } self.decoded.clear(); - self.decoded.reserve(payload_len); + // `payload_len` is attacker-declared and validated only + // against max_block_raw, not bytes remaining, so reserve + // a bounded floor and let extend_from_slice grow as real + // bytes arrive (mirrors lz4's frame-decoder reserve cap). + self.decoded.reserve(payload_len.min(64 * 1024)); self.decoded_idx = 0; self.phase = DecPhase::RawBlock { remaining: payload_len, }; } else { self.block_buf.clear(); - self.block_buf.reserve(payload_len); + // Bounded floor; the CompressedBlock phase grows + // `block_buf` incrementally via extend_from_slice. + self.block_buf.reserve(payload_len.min(64 * 1024)); self.phase = DecPhase::CompressedBlock { block_len: payload_len, gathered: 0, diff --git a/src/lzah/mod.rs b/src/lzah/mod.rs index d7fb102..6731130 100644 --- a/src/lzah/mod.rs +++ b/src/lzah/mod.rs @@ -172,7 +172,12 @@ fn preseed_window() -> [u8; WINDOW] { /// Decode a raw method-5 payload of exactly `expected_len` bytes. fn decode_payload(payload: &[u8], expected_len: usize) -> Result, Error> { - let mut out = Vec::with_capacity(expected_len); + // Cap the eager reservation: `expected_len` is an attacker-controlled + // out-of-band length, so a crafted entry could declare multiple GiB with a + // tiny payload. The `while out.len() < expected_len` loop below enforces the + // true bound, so capping the initial reservation changes only allocation + // behaviour, not correctness. Mirrors the LHA codecs (static_huff/lzhuf). + let mut out = Vec::with_capacity(expected_len.min(1 << 20)); if expected_len == 0 { return Ok(out); } diff --git a/src/lzx/decoder.rs b/src/lzx/decoder.rs index da0f310..e45fe9e 100644 --- a/src/lzx/decoder.rs +++ b/src/lzx/decoder.rs @@ -912,6 +912,15 @@ fn step_huff(ctx: &mut RunCtx) -> Result { if src != ctx.window_pos && src + chunk <= win_size && ctx.window_pos + chunk <= win_size + // Ensure the two sub-ranges are non-overlapping so + // the split_at_mut slices stay in bounds. In the + // wrap case (src > window_pos) this guarantees + // dst_start + chunk <= lo_start; without it the + // a[dst_start..dst_start + chunk] slice runs off the + // end of the lower half (len == src) and panics. + // When this fails we fall through to the correct + // byte-by-byte copy loop below. + && chunk + dist <= win_size { let (lo_start, dst_start) = (src, ctx.window_pos); let (lo, hi) = if lo_start < dst_start { diff --git a/src/rar5/decoder.rs b/src/rar5/decoder.rs index 4d5dfd9..d264b8b 100644 --- a/src/rar5/decoder.rs +++ b/src/rar5/decoder.rs @@ -765,14 +765,20 @@ fn decode_distance(bits: &mut BitBuf, dist_slot: u16, ldc: &Huffman) -> Result= HUFF_DC { return Err(Error::Corrupt); } - let mut dist: u32; + // Accumulate in u64. For slot 63 the base is 0xC0000001 and the + // attacker-supplied high/low bits can push the running total past + // u32::MAX (e.g. 0xC0000001 + 0x3FFFFFF0 + 0xF == 0x1_0000_0000), + // which would overflow a u32 — a debug-build panic and a release-build + // wrap. Working in u64 keeps every intermediate exact; we bound-check + // before narrowing back to the u32 that `emit_match` consumes. + let mut dist: u64; let dbits: u32; if dist_slot < 4 { dbits = 0; - dist = 1 + dist_slot as u32; + dist = 1 + dist_slot as u64; } else { dbits = (dist_slot as u32 / 2) - 1; - dist = 1 + ((2 | (dist_slot as u32 & 1)) << dbits); + dist = 1 + ((2 | (dist_slot as u64 & 1)) << dbits); } if dbits > 0 { if dbits >= 4 { @@ -791,16 +797,23 @@ fn decode_distance(bits: &mut BitBuf, dist_slot: u16, ldc: &Huffman) -> Result window_size` bound on the in-range value. + if dist > u32::MAX as u64 { + return Err(Error::InvalidDistance); + } + Ok(dist as u32) } /// Read a filter descriptor immediately after the main code emits 256. @@ -875,4 +888,28 @@ mod tests { assert_eq!(adjust_length(3, 0x2001), 5); assert_eq!(adjust_length(3, 0x4_0001), 6); } + + /// Regression: slot 63 with maximal attacker-supplied extra/low bits used + /// to overflow the u32 distance accumulator. base = 0xC0000001, the 26 + /// high bits add 0x3FFFFFF0 (→ 0xFFFFFFF1), and the LDC low (15) would + /// push it to 0x1_0000_0000 — an `attempt to add with overflow` panic in + /// the dev profile (overflow-checks on) and a silent wrap in release. + /// The fix accumulates in u64 and rejects distances above u32::MAX with + /// `InvalidDistance` before narrowing. This test panics if unfixed. + #[test] + fn slot_63_max_bits_does_not_overflow() { + // Complete 16-symbol LDC code, every symbol length 4. Canonical + // assignment gives symbol 15 the all-ones code 0b1111, so an + // all-ones bitstream decodes the low nibble to 15 (the maximum). + let ldc = Huffman::from_lengths(&[4u8; HUFF_LDC]).unwrap(); + let mut br = BitBuf::new(); + // 26 high bits + 4 LDC bits = 30 bits; supply 8 bytes of 1s so the + // 16-bit peeks never run dry. + br.reset(&[0xFF; 8], 8); + // Must return an error rather than panicking or wrapping. + assert_eq!( + decode_distance(&mut br, 63, &ldc), + Err(Error::InvalidDistance) + ); + } } diff --git a/src/zstd/decoder.rs b/src/zstd/decoder.rs index 0693236..896528b 100644 --- a/src/zstd/decoder.rs +++ b/src/zstd/decoder.rs @@ -663,7 +663,20 @@ impl Decoder { let seq_data = &block[after_lit..]; let seqs = decode_sequences(seq_data, &mut self.seq_state)?; // 3. LZ77 reconstruction. Append to history. - execute_sequences(&seqs, &lit.literals, &mut self.history)?; + // + // Per RFC 8478 §3.1.1.2 a single Compressed_Block decodes to at most + // Block_Maximum_Size = min(Window_Size, 128 KiB). Enforce that bound on + // the bytes this block appends so a malicious block (e.g. RLE_Mode FSE + // tables emitting huge match-lengths from cheap sequences) can't expand + // `history` to multiple GiB before any output is drained. When no + // Window_Descriptor was present (window_size == 0) fall back to the + // 128 KiB block cap. + let block_max = if self.window_size == 0 { + 128 * 1024 + } else { + core::cmp::min(self.window_size, 128 * 1024) + } as usize; + execute_sequences(&seqs, &lit.literals, &mut self.history, block_max)?; // Return ownership of comp_buf for reuse. self.comp_buf = block; self.comp_buf.clear(); diff --git a/src/zstd/literals.rs b/src/zstd/literals.rs index 2d8ab7c..3b4bc0c 100644 --- a/src/zstd/literals.rs +++ b/src/zstd/literals.rs @@ -21,9 +21,17 @@ use alloc::vec::Vec; use crate::error::Error; use crate::zstd::bitreader::RevBitReader; -use crate::zstd::decoder::MAX_WINDOW_SIZE; use crate::zstd::huffman::{HuffTable, decode_huffman_tree}; +/// Per-block upper bound on a literals section's `Regenerated_Size`. Per RFC +/// 8478 §3.1.1.3.1.1 a block's literals decode to at most Block_Maximum_Size = +/// min(Window_Size, 128 KiB) bytes, which is itself capped at 128 KiB. We don't +/// thread the frame's window size into this module, so we use the unconditional +/// 128 KiB ceiling — far tighter than MAX_WINDOW_SIZE (128 MiB) and enough to +/// stop a few header bytes from eagerly allocating ~1 MiB (RLE) / large Huffman +/// outputs. +const MAX_BLOCK_REGEN_SIZE: u64 = 128 * 1024; + /// State carried across blocks: the most recently seen Huffman tree (used by /// `Treeless_Literals_Block`). #[derive(Default)] @@ -86,9 +94,10 @@ fn decode_raw_or_rle(block: &[u8], is_rle: bool, sf: u8) -> Result MAX_WINDOW_SIZE { + // we materialize it eagerly below. The per-block Regenerated_Size is + // spec-capped at Block_Maximum_Size (<= 128 KiB), so reject anything above + // that before allocating/resizing. + if regen_size as u64 > MAX_BLOCK_REGEN_SIZE { return Err(Error::Corrupt); } @@ -176,9 +185,9 @@ fn decode_compressed_literals( }; // Same decompression-bomb guard as the Raw/RLE path: reject a - // `Regenerated_Size` above the conventional window cap before we allocate - // an output buffer sized to it. - if regen_size as u64 > MAX_WINDOW_SIZE { + // `Regenerated_Size` above Block_Maximum_Size (<= 128 KiB) before we + // allocate an output buffer sized to it. + if regen_size as u64 > MAX_BLOCK_REGEN_SIZE { return Err(Error::Corrupt); } diff --git a/src/zstd/mod.rs b/src/zstd/mod.rs index 3f01f43..4cb9e98 100644 --- a/src/zstd/mod.rs +++ b/src/zstd/mod.rs @@ -102,7 +102,8 @@ pub mod _internal_test_api { let seq_data = &body[lit.consumed..]; let seqs = super::sequences::decode_sequences(seq_data, &mut seq_state)?; let mut out: Vec = Vec::new(); - super::sequences::execute_sequences(&seqs, &lit.literals, &mut out)?; + // Block_Maximum_Size with an unknown window defaults to the 128 KiB cap. + super::sequences::execute_sequences(&seqs, &lit.literals, &mut out, 128 * 1024)?; Ok(out) } diff --git a/src/zstd/sequences.rs b/src/zstd/sequences.rs index 18ad39e..36b71a0 100644 --- a/src/zstd/sequences.rs +++ b/src/zstd/sequences.rs @@ -413,21 +413,46 @@ fn apply_offset(offset_value: u32, literal_length: u32, prev: &mut [u32; 3]) -> /// /// `history` is the previously-decoded output (so back-references can read /// from it); decoded bytes are appended to `history`. +/// +/// `max_block_output` is the per-block decoded-output bound. Per RFC 8478 +/// §3.1.1.2 a single Compressed_Block may decode to at most +/// `Block_Maximum_Size = min(Window_Size, 128 KiB)` bytes. Without this cap a +/// malicious block using RLE_Mode FSE tables (e.g. match-length RLE symbol 52, +/// `ml_base = 65539`, consuming no state bits) emits ~65 KiB per cheap +/// sequence, letting a ~128 KiB input block expand `history` to multiple GiB +/// before any output is drained — a decompression-bomb OOM that bypasses the +/// drained-bytes metering in [`crate::limit::LimitedDecoder`]. We track the +/// bytes this block appends (literals **and** match copies, plus the trailing +/// literals) and abort as soon as the running total would exceed the bound. pub fn execute_sequences( sequences: &[Sequence], literals: &[u8], history: &mut Vec, + max_block_output: usize, ) -> Result<(), Error> { + // Bytes appended to `history` by *this* block so far. `history` itself + // carries earlier blocks' output, so we meter against this running counter + // rather than `history.len()`. + let mut block_output = 0usize; let mut lit_pos = 0usize; for seq in sequences { let ll = seq.literal_length as usize; if lit_pos + ll > literals.len() { return Err(Error::Corrupt); } + let ml = seq.match_length as usize; + // Reject before allocating: a literal-run + match-length that would + // push this block past Block_Maximum_Size is a decompression bomb. + block_output = block_output + .checked_add(ll) + .and_then(|n| n.checked_add(ml)) + .ok_or(Error::Corrupt)?; + if block_output > max_block_output { + return Err(Error::Corrupt); + } history.extend_from_slice(&literals[lit_pos..lit_pos + ll]); lit_pos += ll; let offset = seq.offset as usize; - let ml = seq.match_length as usize; if offset == 0 || offset > history.len() { return Err(Error::Corrupt); } @@ -447,8 +472,14 @@ pub fn execute_sequences( } } } - // Trailing literals: leftover bytes copied verbatim. + // Trailing literals: leftover bytes copied verbatim. They also count + // toward the per-block output bound. if lit_pos < literals.len() { + let trailing = literals.len() - lit_pos; + let total = block_output.checked_add(trailing).ok_or(Error::Corrupt)?; + if total > max_block_output { + return Err(Error::Corrupt); + } history.extend_from_slice(&literals[lit_pos..]); } Ok(()) diff --git a/tests/gzip.rs b/tests/gzip.rs index 7d46022..bda84f5 100644 --- a/tests/gzip.rs +++ b/tests/gzip.rs @@ -435,6 +435,50 @@ fn decode_with_all_optional_fields() { assert_eq!(decoded, b"hello"); } +#[test] +fn decode_with_fhcrc_field() { + // gzip stream of "hello" with only FHCRC set (FLG = 0x02). The 2-byte + // header-CRC field is skipped, not validated. + let mut stream = vec![0x1F, 0x8B, 0x08, 0x02, 0, 0, 0, 0, 0, 0x03]; + stream.extend_from_slice(&[0x12, 0x34]); // FHCRC (skipped) + stream.extend_from_slice(&hex("cb48cdc9c90700")); + stream.extend_from_slice(&[0x86, 0xa6, 0x10, 0x36, 0x05, 0x00, 0x00, 0x00]); + let decoded = decode_chunked(&stream, 1024, 1024).unwrap(); + assert_eq!(decoded, b"hello"); +} + +#[test] +fn decode_with_fextra_and_fhcrc_fields() { + // FEXTRA + FHCRC together (FLG = 0x06). Regression test: the shared + // `aux_idx` counter, left at 2 by the FEXTRA length parse, previously + // caused the FHCRC skip loop to consume 0 bytes, feeding the 2 header-CRC + // bytes into the deflate decoder and corrupting an otherwise valid member. + let mut stream = vec![0x1F, 0x8B, 0x08, 0x06, 0, 0, 0, 0, 0, 0x03]; + stream.extend_from_slice(&[0x04, 0x00]); // XLEN = 4 + stream.extend_from_slice(&[0xAA, 0xBB, 0xCC, 0xDD]); // extra data + stream.extend_from_slice(&[0x12, 0x34]); // FHCRC (skipped) + stream.extend_from_slice(&hex("cb48cdc9c90700")); + stream.extend_from_slice(&[0x86, 0xa6, 0x10, 0x36, 0x05, 0x00, 0x00, 0x00]); + let decoded = decode_chunked(&stream, 1024, 1024).unwrap(); + assert_eq!(decoded, b"hello"); +} + +#[test] +fn decode_with_all_optional_fields_and_fhcrc() { + // FEXTRA + FNAME + FCOMMENT + FHCRC (FLG = 0x1E): exercises the HeaderCrc + // entry path through Comment with `aux_idx` still dirty from FEXTRA. + let mut stream = vec![0x1F, 0x8B, 0x08, 0x1E, 0, 0, 0, 0, 0, 0x03]; + stream.extend_from_slice(&[0x03, 0x00]); // XLEN = 3 + stream.extend_from_slice(&[1, 2, 3]); + stream.extend_from_slice(b"name.txt\0"); + stream.extend_from_slice(b"some comment\0"); + stream.extend_from_slice(&[0x56, 0x78]); // FHCRC (skipped) + stream.extend_from_slice(&hex("cb48cdc9c90700")); + stream.extend_from_slice(&[0x86, 0xa6, 0x10, 0x36, 0x05, 0x00, 0x00, 0x00]); + let decoded = decode_chunked(&stream, 1024, 1024).unwrap(); + assert_eq!(decoded, b"hello"); +} + // ─── malformed-header rejection ───────────────────────────────────────── #[test]