Skip to content

Bound SSZ list pre-allocation - #87

Merged
michaelsproul merged 2 commits into
sigp:mainfrom
eserilev:cap-untrusted-prealloc
Sep 22, 2026
Merged

michaelsproul merged 2 commits into
sigp:mainfrom
eserilev:cap-untrusted-prealloc

Conversation

@eserilev

@eserilev eserilev commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Progressive lists carry no length limit. An SSZ input can claim a huge item count and force a giant Vec::with_capacity pre-allocation before any element is read.

This bounds the pre-allocation in Vec and SmallVec try_from_iter to a fixed 1 MiB. The collection can still grow, so a valid list is never rejected.

@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.40%. Comparing base (258380e) to head (fe462cb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ssz/src/decode/try_from_iter.rs 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
+ Coverage   82.28%   82.40%   +0.11%     
==========================================
  Files          14       14              
  Lines        1225     1233       +8     
==========================================
+ Hits         1008     1016       +8     
  Misses        217      217              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@michaelsproul michaelsproul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this really does anything for the Vec or ProgressiveList case. We will avoid immediately pre-allocating, but AFAICT there's nothing to stop us from consuming the full iterator and allocating the same amount by the time we finish decoding. It helps for oversized length hints, but AFAICT we always derive the length hints from the iterator, not the offsets, so we are not prone to over-allocating anyway.

Further, the decode path for a progressive list of fixed-size items (a common case, including PayloadAttestation/Attestation), doesn't use the TryFromIter impl until after it has already collected into a Vec (which uses the impl From<Iterator<Item = Result<_, _>>> for Result<Vec<_>, _> from the std library):

https://github.com/sigp/ssz_types/blob/3110fa02556d23f9553b1c4ac42414857be82122/src/progressive_variable_list.rs#L275-L279

@eserilev

eserilev commented Sep 21, 2026 •

Copy link
Copy Markdown
Member Author

yeah in the "honest" input case we would consume the full iterator. But the 10MiB network limits help in that case. For example a block with too many attestations still needs to be under the 10MiB limit and decoding that object wouldn't take up too much memory. I think maybe like 50-60mib at worst

From what I understand in decode_list_of_variable_length_items we take item count as first offset divided by four. So a "malicious" input, like some object who claims to have a first offset at 10MiB would pre-allocate a large vec right off the bat. For some types that could cause us to pre-allocate over 1gb in memory. Decoding would fail in that case because the malicious input would end up being junk data so we'd never end up consuming the full iterator. That is really what this PR is protecting against.

For the fixed size case I think thats okay, the size hint is bytes.len() / item_len which is exact

@michaelsproul

Copy link
Copy Markdown
Member

Ohhhhh I missed that. I didn't clock that the iterator was over (1..=num_items) and therefore num_items makes it into the size_hint.

Satisfied that this is correctly patching that gap! Thanks!

@michaelsproul
michaelsproul merged commit 974f78f into sigp:main Sep 22, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants