fix(module): bound section allocations during rwasm decode - #174
Open
dmitry123 wants to merge 1 commit into
Open
fix(module): bound section allocations during rwasm decode#174dmitry123 wants to merge 1 commit into
dmitry123 wants to merge 1 commit into
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 58 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Criterion results (vs baseline)Heads-up: runner perf is noisy; treat deltas as a smoke check. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
d1r1
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes FLU-1096.
Problem
Every section of the rwasm binary format starts with a
u64length read straight from untrusted input. That length went directly intoVec::with_capacity, so an 11-byte binary could request an arbitrary allocation before a single element was read:u64::MAX→capacity overflowpanic inraw_vec2^40→ ~8 TiB request →handle_alloc_error→ abort, which nocatch_unwindcan containReachable from every decode entry point (
new,new_checked,new_checked_exact,new_verified,new_verified_exact), because the allocation happens during decoding, before verification runs.Scope correction
The issue attributed this to the hand-written
Decodeimpl forInstructionSetbypassing bincode'sclaim_container_readguard, and expected the derived sibling fields to be safe. That turned out not to hold:claim_container_readis a no-op unless the config sets a decode limit, and the module is decoded withbincode::config::legacy(), which isNoLimit. bincode's ownVecdecoder then reachesVec::with_capacity(len)/vec![0u8; len]unguarded.Verified against
devel— all four sections panic identically:code_sectioncapacity overflowUnexpectedEnd { additional: 4 }data_sectioncapacity overflowUnexpectedEnd { additional: 4096 }elem_sectioncapacity overflowUnexpectedEnd { additional: 4 }hint_sectioncapacity overflowUnexpectedEnd { additional: 4096 }So the fix covers all four rather than
code_sectionalone.Fix
New
src/types/codec.rsholds allocation-safe primitives for length-prefixed sections, used byInstructionSet::decodeandRwasmModuleInner::decode:decode_section_lengthconverts theu64prefix throughusize::try_from, returningOutsideUsizeRangeinstead of truncating on 32-bit targets.decode_section_vecreserves at most 4096 elements up front and grows on demand per element, so peak memory tracks the input actually present. It also performs theclaim_container_read/unclaim_bytes_readaccounting, so the code section now honours a decode limit if one is ever configured.decode_section_bytesreserves and reads byte sections in 64 KiB chunks, reading straight into the vector's tail rather than through a temporary buffer.A truncated section now fails with
UnexpectedEndthe moment the reader runs dry, with no allocation proportional to the claimed length.Not a size limit
The issue also suggested pairing this with an explicit
N_MAX_CODE_SECTION_LENcap checked inverify_module. I deliberately left that out: picking a maximum module/section size is a consensus-affecting policy decision that would reject binaries which decode today, and it is not needed to close the DoS — bounding the allocation by the available input is sufficient and behaviour-preserving.docs/security-considerations.md:42still lists "enforce module/section size limits" as an open mitigation, and it is worth its own issue.Tests
test_decode_rejects_oversized_section_lengths— all four sections ×{u64::MAX, 2^40}, asserting a cleanDecodeErrorinstead of a panic or abort.test_decode_accepts_large_hint_section— a 512 KiB hint section still round-trips, guarding against the fix turning into a size cap.Full suite green (158 tests),
clippy --all-targets --all-featuresclean,--no-default-featuresbuilds. The remainingcargo fmt --checkdiffs (src/lib.rs,src/types/mod.rs:58,src/vm/memory.rs:57) pre-exist ondeveland are untouched here.