harden(decode): uniform FrameContentSizeMismatch on Compressed overshoot

zstd/fuzz/Cargo.toml

@@ -26,6 +26,10 @@ members = ["."]
 name = "decode"
 path = "fuzz_targets/decode.rs"
 
+[[bin]]
+name = "decode_direct"
+path = "fuzz_targets/decode_direct.rs"
+
 [[bin]]
 name = "encode"
 path = "fuzz_targets/encode.rs"

zstd/fuzz/fuzz_targets/decode_direct.rs

@@ -0,0 +1,26 @@
+#![no_main]
+#[macro_use]
+extern crate libfuzzer_sys;
+
+use structured_zstd::decoding::FrameDecoder;
+
+// Direct-decode (`FrameDecoder::decode_all` into a fixed-capacity slice)
+// adversarial-input harness for #246. The existing `decode` target only
+// fuzzes the growable `StreamingDecoder` path (FlatBuf / RingBuffer, which
+// can never overflow because the backing Vec grows on demand). The direct
+// path routes through `UserSliceBackend`, whose writes are bounded by the
+// caller-provided slice — a malformed frame that expands past the declared
+// `frame_content_size` MUST surface a structured error, never panic or
+// write out of bounds. This target drives exactly that path.
+fuzz_target!(|data: &[u8]| {
+    // A fixed output slice forces the direct path when the frame's
+    // declared content size fits. Cap at 1 MiB so an adversarial frame
+    // declaring a huge `frame_content_size` can't OOM the fuzzer via the
+    // up-front output allocation; frames larger than this fall back to the
+    // growable path (still panic-free) or are rejected. The decoder's own
+    // window cap (100 MiB) and the per-block fallible write surface are
+    // what we're exercising — any panic here is a bug.
+    let mut out = vec![0u8; 1 << 20];
+    let mut dec = FrameDecoder::new();
+    let _ = dec.decode_all(data, &mut out);
+});

zstd/src/decoding/buffer_backend.rs

@@ -387,14 +387,21 @@ pub(crate) trait BufferBackend: Sized {
     //
     // Parallel `try_*` methods that return `Err(BackendOverflow)`
     // instead of panicking when the write would exceed the backend's
-    // capacity. Currently wired on Raw and RLE block paths only;
-    // Compressed-block sequence execution still uses the panic-on-
-    // overflow unchecked writes and will be migrated in a follow-up.
-    // Used by the direct-decode path (`decode_all` +
-    // descendants) so a malformed Raw/RLE block whose declared
-    // decompressed payload exceeds the caller-provided output slice
-    // surfaces as a structured `FrameDecoderError::FrameContentSizeMismatch`
-    // instead of an abort.
+    // capacity. Wired across EVERY direct-decode write path: Raw / RLE
+    // blocks (`try_extend` / `try_extend_and_fill`), the Compressed
+    // block's sequence executor (`exec_sequence_inline` returns
+    // `Result`, the fallback chain uses `try_push` +
+    // `repeat_lookahead_prefetched`, tail literals use `try_push`), and
+    // the match-repeat pre-check (`try_reserve`). Used by the
+    // direct-decode path (`decode_all` + descendants) so a malformed
+    // block whose decompressed payload exceeds the caller-provided
+    // output slice surfaces as a structured
+    // `FrameDecoderError::FrameContentSizeMismatch` instead of an abort
+    // — uniformly for Raw, RLE, and Compressed blocks (see
+    // `FrameDecoder::run_direct_decode`, which folds the Compressed
+    // sequence-executor `OutputBufferOverflow` into the same
+    // `FrameContentSizeMismatch` contract as the Raw/RLE
+    // `BackendOverflow` arm).
     //
     // The growable backends (`FlatBuf`, `RingBuffer`) rely on the
     // default impls below — they delegate to the corresponding
@@ -455,12 +462,16 @@ pub(crate) trait BufferBackend: Sized {
     /// satisfy the `Self::extend_from_within_unchecked` safety
     /// contract at the call site.
     ///
-    /// NOTE: Currently unused on production paths. The direct
-    /// decode's Compressed-block sequence executor writes via the
-    /// existing unchecked path; threading `try_*` through the
-    /// fused decode+execute pipeline is the next step toward
-    /// unconditional adversarial-input safety. RLE/Raw blocks
-    /// already use `try_extend_and_fill` / `try_extend`.
+    /// NOTE: Retained for the SAFE-surface test matrix and as the
+    /// wrap-aware reference impl; not called on production paths
+    /// (hence `#[allow(dead_code)]`). The Compressed-block direct path
+    /// is already DoS-safe WITHOUT this method: its match-repeat copies
+    /// go through `DecodeBuffer::repeat_lookahead_prefetched`, which
+    /// pre-checks capacity via [`Self::try_reserve`] before the
+    /// unchecked wildcopy, and its literal+match sequence copies go
+    /// through `exec_sequence_inline` (returns `Result`). RLE/Raw use
+    /// `try_extend_and_fill` / `try_extend`. So every adversarial
+    /// overshoot already surfaces as a structured error.
     #[allow(dead_code)]
     fn try_extend_from_within(&mut self, start: usize, len: usize) -> Result<(), BackendOverflow> {
         // Default impl: a SAFE method must NOT delegate to the
@@ -541,9 +552,12 @@ pub(crate) trait BufferBackend: Sized {
 /// donor-inline paths inside the sequence executor) or
 /// `DecodeBufferError::OutputBufferOverflow` (the match-repeat
 /// `try_reserve` pre-check inside `DecodeBuffer::repeat_inner`).
-/// Both bubble up as a structured `FrameDecoderError` (typically
-/// wrapped in `FailedToReadBlockBody`) — callers never see
-/// `BackendOverflow` directly.
+/// On the direct-decode path both are folded by
+/// `FrameDecoder::run_direct_decode` into
+/// `FrameDecoderError::FrameContentSizeMismatch` — the same
+/// caller-visible error a Raw / RLE overshoot yields, so the
+/// "content exceeded declared size" contract is uniform across block
+/// types. Callers never see `BackendOverflow` directly.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct BackendOverflow {
     /// Current physical write cursor at the moment the write was

zstd/src/decoding/errors.rs

@@ -1020,6 +1020,32 @@ impl From<crate::decoding::buffer_backend::BackendOverflow> for ExecuteSequences
     }
 }
 
+impl ExecuteSequencesError {
+    /// `Some(requested)` when this error is a fixed-capacity output-buffer
+    /// overshoot from the Compressed-block sequence executor — either the
+    /// donor-inline path ([`Self::OutputBufferOverflow`]) or the
+    /// match-repeat fallback ([`DecodeBufferError::OutputBufferOverflow`]
+    /// wrapped in [`Self::DecodebufferError`]). `requested` is the byte
+    /// count the failing write tried to append past the slice end.
+    ///
+    /// `None` for every non-overflow variant. Used by
+    /// `FrameDecoder::run_direct_decode` to fold an in-block Compressed
+    /// overshoot into the same `FrameContentSizeMismatch` contract the
+    /// Raw/RLE [`DecodeBlockContentError::BackendOverflow`] arm already
+    /// produces — both mean "the frame's content expands past the
+    /// declared `frame_content_size`".
+    pub(crate) fn output_overflow_requested(&self) -> Option<usize> {
+        match self {
+            ExecuteSequencesError::OutputBufferOverflow { requested, .. } => Some(*requested),
+            ExecuteSequencesError::DecodebufferError(DecodeBufferError::OutputBufferOverflow {
+                requested,
+                ..
+            }) => Some(*requested),
+            _ => None,
+        }
+    }
+}
+
 #[derive(Debug)]
 #[non_exhaustive]
 pub enum DecodeSequenceError {
@@ -1545,10 +1571,46 @@ mod tests {
     use alloc::{string::ToString, vec};
 
     use super::{
-        BlockTypeError, DecodeBlockContentError, DecodeSequenceError, DecompressBlockError,
-        DecompressLiteralsError, FSETableError, FrameDecoderError, HuffmanTableError,
+        BlockTypeError, DecodeBlockContentError, DecodeBufferError, DecodeSequenceError,
+        DecompressBlockError, DecompressLiteralsError, ExecuteSequencesError, FSETableError,
+        FrameDecoderError, HuffmanTableError,
     };
 
+    #[test]
+    fn execute_sequences_output_overflow_requested_covers_all_arms() {
+        // #246: `run_direct_decode` folds a Compressed-block overshoot into
+        // `FrameContentSizeMismatch` by reading `requested` from whichever
+        // overflow shape the executor produced. Cover all three arms:
+        //   1. donor-inline path -> `OutputBufferOverflow` directly,
+        //   2. match-repeat path -> `DecodebufferError(OutputBufferOverflow)`,
+        //   3. any other variant -> None (no fold).
+        let inline = ExecuteSequencesError::OutputBufferOverflow {
+            tail: 10,
+            requested: 7,
+            capacity: 12,
+        };
+        assert_eq!(inline.output_overflow_requested(), Some(7));
+
+        let repeat =
+            ExecuteSequencesError::DecodebufferError(DecodeBufferError::OutputBufferOverflow {
+                tail: 3,
+                requested: 99,
+                capacity: 4,
+            });
+        assert_eq!(repeat.output_overflow_requested(), Some(99));
+
+        // Non-overflow variants (and non-overflow DecodeBufferError) -> None.
+        assert_eq!(
+            ExecuteSequencesError::ZeroOffset.output_overflow_requested(),
+            None
+        );
+        assert_eq!(
+            ExecuteSequencesError::DecodebufferError(DecodeBufferError::ZeroOffset)
+                .output_overflow_requested(),
+            None
+        );
+    }
+
     #[test]
     fn block_and_sequence_display_messages_are_specific() {
         assert_eq!(

zstd/src/decoding/frame_decoder.rs

@@ -2101,6 +2101,32 @@ impl FrameDecoder {
                             .saturating_add(u64::from(block_header.decompressed_size)),
                     });
                 }
+                // Compressed-block in-block overshoot: the sequence
+                // executor (donor-inline path) or the match-repeat
+                // fallback tripped the fixed-capacity backend's per-write
+                // check. Unlike Raw/RLE, a Compressed block carries no
+                // header-declared output size, so `produced` is computed
+                // from the partial fill: `tail` bytes were written before
+                // the failing op, and `requested` is what overflowed —
+                // their sum is a strict lower bound on the frame's true
+                // expanded size and is always > `content_size` (the
+                // direct path is only entered when the slice is sized to
+                // `content_size + WILDCOPY_OVERLENGTH`, so any overflow
+                // means the frame exceeded the declared FCS, never a
+                // caller-undersized buffer). Folds into the same
+                // `FrameContentSizeMismatch` contract as Raw/RLE.
+                Err(crate::decoding::errors::DecodeBlockContentError::DecompressBlockError(
+                    crate::decoding::errors::DecompressBlockError::ExecuteSequencesError(ref e),
+                )) if e.output_overflow_requested().is_some() => {
+                    let requested = e
+                        .output_overflow_requested()
+                        .expect("guard guarantees Some") as u64;
+                    let tail = direct.buffer.buffer_ref().tail() as u64;
+                    return Err(err::FrameContentSizeMismatch {
+                        declared: content_size,
+                        produced: tail.saturating_add(requested),
+                    });
+                }
                 Err(e) => {
                     return Err(block_body_decode_error(
                         e,
@@ -2420,6 +2446,81 @@ mod tests {
         );
     }
 
+    #[test]
+    fn decode_all_compressed_block_fcs_overflow_returns_structured_error() {
+        // Acceptance test for #246: a malformed frame whose *Compressed*
+        // block expands past the declared `frame_content_size` must
+        // surface `FrameContentSizeMismatch` from the direct-decode path
+        // (UserSliceBackend sequence executor), NOT panic and NOT a
+        // generic FailedToReadBlockBody. The Raw-block sibling above
+        // covers the `BackendOverflow` arm; this covers the Compressed
+        // sequence-executor overflow arm (`ExecuteSequencesError::
+        // OutputBufferOverflow` folded into FrameContentSizeMismatch in
+        // `run_direct_decode`).
+        //
+        // Construction: compress a compressible payload to get a genuine
+        // Compressed block + a header-declared FCS, then surgically patch
+        // the FCS field down to a tiny value. The block body still
+        // decodes (literals/sequences are independent of FCS) and the
+        // sequence executor overflows the small output slice.
+        // Highly compressible payload (repeated phrase) → Compressed
+        // block whose sequence executor produces ~4 KiB of output.
+        let unit = b"The quick brown fox jumps over the lazy dog. ";
+        let mut payload = Vec::with_capacity(4 * 1024);
+        while payload.len() < 4 * 1024 {
+            payload.extend_from_slice(unit);
+        }
+        let mut compressor = FrameCompressor::new(CompressionLevel::Default);
+        compressor.set_source(payload.as_slice());
+        let mut frame = Vec::new();
+        compressor.set_drain(&mut frame);
+        compressor.compress();
+        // Sanity: the encoder actually compressed (=> a Compressed block,
+        // not a raw-stored fallback) so we exercise the sequence path.
+        assert!(frame.len() < payload.len());
+
+        // Locate the FCS field: it is the last `fcs_len` bytes of the
+        // frame header, whose total size `header_size` includes the magic.
+        // A ~4 KiB single-segment frame declares FCS = 4096, which lands in
+        // the 2-byte field range [256, 65791] (RFC 8878 §3.1.1.1.4) — assert
+        // that so the patch logic below stays a single deterministic branch.
+        let (header, header_size) =
+            super::super::frame::read_frame_header(frame.as_slice()).expect("valid header");
+        let fcs_len = header
+            .descriptor
+            .frame_content_size_bytes()
+            .expect("fcs present") as usize;
+        assert_eq!(
+            fcs_len, 2,
+            "4 KiB single-segment frame must use a 2-byte FCS"
+        );
+        let fcs_off = header_size as usize - fcs_len;
+
+        // Patch the 2-byte FCS to its floor: stored bytes 0 decode to 256
+        // (the field's `+256` bias), far below the 4 KiB the block actually
+        // produces, so the sequence executor overflows the output slice.
+        let patched_declared: u64 = 256;
+        frame[fcs_off] = 0;
+        frame[fcs_off + 1] = 0;
+
+        // Size the output to declared + WILDCOPY slack so the direct path
+        // is eligible (output.len() >= content_size + slack) — the
+        // overflow then comes from the frame, not an undersized buffer.
+        let slack = super::super::buffer_backend::WILDCOPY_OVERLENGTH;
+        let mut out = alloc::vec![0u8; patched_declared as usize + slack];
+        let mut dec = FrameDecoder::new();
+        let err = dec
+            .decode_all(frame.as_slice(), &mut out)
+            .expect_err("Compressed block exceeding FCS must fail decode");
+        match err {
+            super::FrameDecoderError::FrameContentSizeMismatch { declared, produced } => {
+                assert_eq!(declared, patched_declared, "declared echoes patched FCS");
+                assert!(produced > declared, "produced must exceed declared");
+            }
+            other => panic!("expected FrameContentSizeMismatch, got {other:?}"),
+        }
+    }
+
     /// Block-precise error positions (#174): a failing block header / body
     /// reports its 0-based index and frame-absolute offset, consistent with
     /// the encoder's `FrameEmitInfo.blocks[index].offset_in_frame`.

zstd/src/decoding/user_slice_buf.rs

@@ -101,11 +101,14 @@ use super::buffer_backend::{BufferBackend, WILDCOPY_OVERLENGTH};
 /// `exec_sequence_inline` (which returns
 /// `Result<(), ExecuteSequencesError>`). A malformed Compressed
 /// block whose payload expands past the declared
-/// `frame_content_size` surfaces as
+/// `frame_content_size` is caught per-write as
 /// `ExecuteSequencesError::OutputBufferOverflow` (literal-push /
 /// donor-inline path) or `DecodeBufferError::OutputBufferOverflow`
-/// (match-repeat path), both of which propagate up the call stack
-/// as a structured `FrameDecoderError` instead of panicking.
+/// (match-repeat path); `FrameDecoder::run_direct_decode` folds both
+/// into a structured `FrameDecoderError::FrameContentSizeMismatch` —
+/// the SAME error a Raw / RLE overshoot produces — instead of
+/// panicking. The contract is uniform across all three block types:
+/// "the frame's content exceeded its declared size".
 ///
 /// The INFALLIBLE entry points (`extend`, `extend_and_fill`,
 /// `extend_from_within_unchecked`) remain on the type as defense in
@@ -1146,6 +1149,41 @@ mod tests {
     /// callee. Tests cover: short-literal + short-match, long
     /// literal (wildcopy tail), short-offset match (overlapCopy8 +
     /// 8-byte stride), long-offset match (wildcopy_no_overlap).
+    #[test]
+    fn exec_sequence_inline_overflow_returns_output_buffer_overflow() {
+        // #246: the donor-inline sequence executor must return
+        // `ExecuteSequencesError::OutputBufferOverflow` (NOT panic, NOT
+        // an out-of-bounds unsafe write) when a sequence's literal+match
+        // copy plus wildcopy overshoot would land past the fixed slice.
+        // This is the per-sequence guard that keeps the unsafe write
+        // surface inside the user slice on the way to the post-block FCS
+        // check; the higher-level acceptance test
+        // (`decode_all_compressed_block_fcs_overflow_returns_structured_error`)
+        // proves the whole chain folds into `FrameContentSizeMismatch`.
+        use super::super::errors::ExecuteSequencesError;
+        // Tiny slice: 16-byte capacity, tail already at 8. A sequence
+        // writing 8 literals + 8 match (16 bytes) plus the 15-byte
+        // wildcopy overshoot cannot fit in `16 - 8 = 8` remaining bytes.
+        let mut buf = vec![0u8; 16];
+        for (i, slot) in buf.iter_mut().take(8).enumerate() {
+            *slot = i as u8;
+        }
+        let mut b = UserSliceBackend::from_slice(&mut buf);
+        b.tail = 8;
+        let lits = [0xAAu8; 16];
+        // SAFETY: `lits` is a 16-byte parent buffer (donor 16-byte read
+        // slack satisfied); the call must return Err before any write
+        // past the slice end.
+        let err = unsafe { b.exec_sequence_inline(lits.as_ptr(), 8, 4, 8) }
+            .expect_err("overshoot must return OutputBufferOverflow");
+        assert!(
+            matches!(err, ExecuteSequencesError::OutputBufferOverflow { .. }),
+            "expected OutputBufferOverflow, got {err:?}"
+        );
+        // Backend left untouched on Err — tail not advanced.
+        assert_eq!(b.tail, 8, "tail must not advance on overflow");
+    }
+
     #[test]
     fn exec_sequence_inline_short_literal_plus_long_offset_match() {
         // Layout: pre-fill `tail = 8` with a "history" region so