diff --git a/crates/ssz-types/src/progressive_list.rs b/crates/ssz-types/src/progressive_list.rs index b909011..9938ba5 100644 --- a/crates/ssz-types/src/progressive_list.rs +++ b/crates/ssz-types/src/progressive_list.rs @@ -130,3 +130,25 @@ impl HashTreeRoot for ProgressiveList { mix_in_length(hasher, &root, length) } } + +#[cfg(test)] +mod tests { + use super::*; + + // `ProgressiveList` has no max length, so it relies entirely on the core + // decoder rejecting an out-of-bounds first offset before reserving capacity. + // Without that guard, a few bytes could force a multi-gigabyte allocation. + #[test] + fn decode_rejects_oversized_first_offset_before_alloc() { + // first_offset = 0xFFFF_FFFC (a multiple of 4) in an 8-byte buffer. + let bytes = [0xFC, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00]; + let result = ProgressiveList::>::from_ssz_bytes(&bytes); + assert_eq!( + result, + Err(DecodeError::OffsetOutOfBounds { + offset: 0xFFFF_FFFC, + length: 8, + }) + ); + } +} diff --git a/crates/ssz/src/decode.rs b/crates/ssz/src/decode.rs index 8b1926d..3c59595 100644 --- a/crates/ssz/src/decode.rs +++ b/crates/ssz/src/decode.rs @@ -369,6 +369,12 @@ fn decode_variable_length_items_with_max( }); } + // Reject an out-of-bounds first offset *before* deriving `num_items` and + // reserving capacity below. `first_offset` is attacker-controlled (it is the + // declared start of the element data), so without this guard a few input + // bytes could request a multi-gigabyte `Vec::with_capacity` and OOM the + // process. The per-offset loop also checks this, but only after allocating — + // keep this early check (do not "deduplicate" it away). if first_offset > bytes.len() { return Err(DecodeError::OffsetOutOfBounds { offset: first_offset, diff --git a/crates/ssz/tests/encode_decode.rs b/crates/ssz/tests/encode_decode.rs index 610027a..df197d9 100644 --- a/crates/ssz/tests/encode_decode.rs +++ b/crates/ssz/tests/encode_decode.rs @@ -547,3 +547,25 @@ fn container_encoder_new() { enc2.finalize(); assert_eq!(buf1, buf2); } + +// ── DoS regression: bounded pre-allocation in variable-length list decode ── + +// A variable-length list reads `num_items` from the first 4-byte offset +// (attacker-controlled) and reserves `Vec::with_capacity(num_items + 1)`. The +// decoder must reject an out-of-bounds first offset before that allocation, +// otherwise a few input bytes can force a multi-gigabyte allocation (OOM). +// This covers the *uncapped* `Vec` path (also used by `ProgressiveList`), +// which has no max-length to fall back on. +#[test] +fn variable_list_rejects_oversized_first_offset_before_alloc() { + // first_offset = 0xFFFF_FFFC (~4.29e9, a multiple of 4) in an 8-byte buffer. + let bytes = [0xFC, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00]; + let result = Vec::>::from_ssz_bytes(&bytes); + assert_eq!( + result, + Err(DecodeError::OffsetOutOfBounds { + offset: 0xFFFF_FFFC, + length: 8, + }) + ); +} diff --git a/fuzz/fuzz_targets/offset_adversarial.rs b/fuzz/fuzz_targets/offset_adversarial.rs index b2d6a76..28c1c8f 100644 --- a/fuzz/fuzz_targets/offset_adversarial.rs +++ b/fuzz/fuzz_targets/offset_adversarial.rs @@ -34,6 +34,15 @@ struct FixedThenVar { data: Vec, } +// Variable-length collection of *variable-size* elements. This reaches the +// offset-table allocation path (`Vec::with_capacity(num_items)`) that the +// structs above never hit — their fields are fixed-size `u8` elements. That +// gap previously hid a pre-allocation DoS in variable-element list decoding. +#[derive(Debug, PartialEq, SszEncode, SszDecode)] +struct VarItemList { + items: Vec>, +} + fuzz_target!(|data: &[u8]| { // Attempt decode of each struct — must never panic regardless of input. // If decode succeeds, roundtrip must hold. @@ -61,4 +70,18 @@ fuzz_target!(|data: &[u8]| { let decoded = FixedThenVar::from_ssz_bytes(&encoded).unwrap(); assert_eq!(decoded, v, "FixedThenVar roundtrip"); } + + if let Ok(v) = VarItemList::from_ssz_bytes(data) { + let encoded = v.to_ssz(); + let decoded = VarItemList::from_ssz_bytes(&encoded).unwrap(); + assert_eq!(decoded, v, "VarItemList roundtrip"); + } + + // The bare uncapped `Vec` of variable-size elements (also the basis for + // `ProgressiveList`): the offset-table allocation has no max-length backstop. + if let Ok(v) = Vec::>::from_ssz_bytes(data) { + let encoded = v.to_ssz(); + let decoded = Vec::>::from_ssz_bytes(&encoded).unwrap(); + assert_eq!(decoded, v, "Vec> roundtrip"); + } });