lambdaclass / lambdaclass/libssz
Harden test/fuzz coverage for the pre-allocation DoS guard (follow-up to #24)
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 10
- Forks
- 4
- PR merge metrics
- No merged PRs in 30d
Description
#24 fixes the pre-allocation OOM by checking list length before allocating, and adds tests that a capped SszList rejects over-capacity input before decoding item bodies (via panic sentinels). Two coverage gaps remain that would let a future refactor silently reintroduce the issue:
1. No regression test for the uncapped path. The panic-sentinel tests only exercise the capped path (max_len = N). The most severe variant is the uncapped one — Vec<T> and ProgressiveList<T> decode through decode_list_with_max(_, usize::MAX), where the first_offset > bytes.len() guard is the only thing bounding the Vec::with_capacity(num_items + 1) allocation. Nothing tests it, so moving or weakening that guard would pass all current tests while restoring a ~10^6x allocation amplification (a few input bytes -> multi-GB).
Suggested test:
#[test]
fn variable_list_rejects_oversized_first_offset_before_alloc() {
let bytes = [0xFC, 0xFF, 0xFF, 0xFF, 0, 0, 0, 0]; // first_offset = 0xFFFFFFFC
assert_eq!(
Vec::<Vec<u8>>::from_ssz_bytes(&bytes),
Err(DecodeError::OffsetOutOfBounds { offset: 0xFFFFFFFC, length: 8 }),
);
}
2. Fuzz blind spot. The offset_adversarial fuzz target only uses Vec<u8> / u64 fields, so it never reaches the variable-element offset-table allocation — which is why this class wasn't caught. Adding a variable-element collection (e.g. a Vec<Vec<u8>> field, or an SszList<Vec<u8>, N>) restores coverage.
3. (Minor) WHY comment on the first_offset > bytes.len() check — it looks redundant with the per-offset check inside the loop, so note it's the pre-allocation guard to deter a future dedup.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the existing offset_adversarial fuzz target and the capped SszList panic-sentinel tests. Add coverage for the uncapped Vec<Vec> path and a variable-element collection in the fuzz target, then document why the first_offset guard must remain; done means oversized offsets fail before allocation and fuzzing reaches the offset-table path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100