lambdaclass / lambdaclass/libssz

Harden test/fuzz coverage for the pre-allocation DoS guard (follow-up to #24)

Open
#25 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.