From c9ca68e6b6e3ebc4c5a5880d3f0f33b3b30c7224 Mon Sep 17 00:00:00 2001 From: Geoffrey Oxberry Date: Fri, 22 May 2026 19:11:18 +0000 Subject: [PATCH] fix(payload): reject zero-byte arbitrary input for Block `Block::total_bytes` is `NonZeroU32`, but the `#[cfg(feature = "arbitrary")] impl Arbitrary for Block` previously called `NonZeroU32::new(u32::arbitrary(u)?).expect("...")`. When the fuzzer happened to produce a zero (which it can, by contract), the expect panicked instead of rejecting the input. The fix uses `.ok_or(arbitrary::Error::IncorrectFormat)?`, the standard arbitrary-crate signal for "this input shape is invalid; skip it and try another." The `bytes` allocation is also moved after the rejection check so a rejected input no longer allocates first. Adds `arbitrary_rejects_zero_total_bytes` as a regression test under `#[cfg(all(test, feature = "arbitrary"))]`. The test was written against the old code and panicked there; against the fixed code it passes by matching `Err(arbitrary::Error::IncorrectFormat)`. Reachable from `lading_fuzz`, which enables the `arbitrary` feature. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 8 ++++++++ lading_payload/src/block.rs | 32 +++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc7650805..dfea01763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +- Fixed a latent panic in `lading_payload::block::Block::arbitrary` + (`#[cfg(feature = "arbitrary")]`). `u32::arbitrary` can legitimately + produce 0, but `Block::total_bytes` is `NonZeroU32`; the previous code + called `NonZeroU32::new(total_bytes).expect("total_bytes must be + non-zero")` and panicked on the zero case. The fix returns + `arbitrary::Error::IncorrectFormat` so the fuzzer rejects the input + and moves on. A regression test (`arbitrary_rejects_zero_total_bytes`) + pins the new behavior. - Annotated 11 `lading_payload` functions that intentionally panic on invariant violations with `#[expect(clippy::expect_used, reason = "...")]`. Covered: `block::Cache::read_at` (documented `usize`/`u64` overflow diff --git a/lading_payload/src/block.rs b/lading_payload/src/block.rs index 1684b1068..990f7a454 100644 --- a/lading_payload/src/block.rs +++ b/lading_payload/src/block.rs @@ -117,10 +117,16 @@ pub enum ConstructBlockCacheError { #[cfg(feature = "arbitrary")] impl<'a> arbitrary::Arbitrary<'a> for Block { fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { - let total_bytes = u32::arbitrary(u)?; - let bytes = u.bytes(total_bytes as usize).map(Bytes::copy_from_slice)?; + // `u32::arbitrary` can legitimately produce 0, but `Block::total_bytes` + // is `NonZeroU32`. Reject zero-byte inputs via `IncorrectFormat` so the + // fuzzer skips them and tries another input. + let total_bytes = + NonZeroU32::new(u32::arbitrary(u)?).ok_or(arbitrary::Error::IncorrectFormat)?; + let bytes = u + .bytes(total_bytes.get() as usize) + .map(Bytes::copy_from_slice)?; Ok(Self { - total_bytes: NonZeroU32::new(total_bytes).expect("total_bytes must be non-zero"), + total_bytes, bytes, metadata: BlockMetadata::default(), }) @@ -766,3 +772,23 @@ where }) } } + +#[cfg(all(test, feature = "arbitrary"))] +mod arbitrary_tests { + use super::Block; + use arbitrary::{Arbitrary, Unstructured}; + + /// Regression: prior to the fix, `Block::arbitrary` could panic via + /// `NonZeroU32::new(0).expect(...)` whenever `u32::arbitrary` produced 0. + /// The fixed version must reject zero-byte inputs via + /// `arbitrary::Error::IncorrectFormat` so the fuzzer skips them. + #[test] + fn arbitrary_rejects_zero_total_bytes() { + let data = [0u8; 16]; + let mut u = Unstructured::new(&data); + match Block::arbitrary(&mut u) { + Err(arbitrary::Error::IncorrectFormat) => {} + other => panic!("expected IncorrectFormat, got {other:?}"), + } + } +}