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:?}"), + } + } +}