ethereum / ethereum/sonobe

[L-2] Public input concatenation enables memory DoS

Open
#239 2 comments 1 reaction 1 assignee Claimed by @winderica View on GitHub
decider security
Dominant language
Rust
Stars
285
Forks
97
PR merge metrics
No merged PRs in 30d

Description

**Context**: `folding-schemes/src/folding/nova/decider.rs`

### Description:

`verify()` takes `z_0` and `z_i` as owned `Vec`s and then builds `c1_public_input`/`c2_public_input` via `.concat()`, which allocates and copies all elements into new vectors.
If `verify()` is reachable with attacker-controlled `z_0`/`z_i` sizes, this can cause large allocations and excessive copying (memory/CPU DoS) even when the proof is invalid.

**Exploit path**: a verifier service that accepts user-supplied `z_0`/`z_i` (or deserializes them from user input) can be forced to allocate/copy very large buffers by sending oversized arrays.

**Impacted code**
```rust
let c1_public_input = [
&[vp.pp_hash, i][..],
&z_0,
&z_i,
&U_final_commitments.inputize_nonnative(),
&cf_U.inputize_nonnative(),
&proof.cs1_challenges,
&proof.cs1_proofs.iter().map(|p| p.eval).collect::>(),
&proof.cmT.inputize_nonnative(),
]
.concat();

let c2_public_input: Vec = [
&[pp_hash_Fq][..],
&cf_U.inputize(),
&proof.cs2_challenges,
&proof.cs2_proofs.iter().map(|p| p.eval).collect::>(),
]
.concat();
```

### Recommendation

Avoid `.concat()` over attacker-sized vectors.
Instead, pre-check expected public input length from the SNARK verifying keys (e.g., Groth16 `vk.gamma_abc_g1.len() - 1`) and reject mismatched sizes early, then build the public input with `Vec::with_capacity(expected)` and `extend_from_slice` to avoid repeated allocations/copies.

Consider taking `z_0`/`z_i` as slices (`&[C1::ScalarField]`) rather than owned `Vec`s.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.