fix(#76)!: checked size arithmetic in MatterBuilder variable-size parse#77
Merged
Conversation
The qb64/qb2 variable-size parse paths computed the frame size from the attacker-controlled decoded soft `size` with unchecked arithmetic (`fs = size * 4 + cs`, `bfs = ceil(fs * 3 / 4)`), violating the arithmetic-safety mandatory rule. In debug this panics on overflow (DoS on untrusted input); in release it wraps to a small bogus `fs` that then slices a truncated frame as valid (silently-wrong parse). Extract the three sites (from_qualified_base64 :132, from_qualified_base2 :263/:265) into checked helpers `compute_full_size` / `compute_qb2_byte_size` using checked_mul/checked_add, returning a new typed `ValidationError::SizeOverflow` on overflow. BREAKING CHANGE: `ValidationError` gains a `SizeOverflow` variant; as the enum is public and not `#[non_exhaustive]`, downstream exhaustive matches must handle it. MINOR under the 0.x SemVer convention. Latent, not reachable through the parse API today: variable codes cap the soft field at ss=4, so size <= 2^24-1 and size*4 fits even 32-bit usize (wasm32). Fixed as a rule violation + defense-in-depth. Tests probe the checked helpers directly (RED: bare arithmetic panics on overflow; GREEN: typed Err) since the overflow is unreachable via from_qualified_base64/base2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Closes #76.
Problem
MatterBuilder's variable-size parse paths computed the frame size from the attacker-controlled decoded softsizeusing unchecked arithmetic, violating the arithmetic-safety mandatory rule:from_qualified_base64(:132) andfrom_qualified_base2(:263):fs = (size * 4) + csfrom_qualified_base2(:265):bfs = (fs * 3).div_ceil(4)sizecomes fromdecode_int(soft). In debug this panics on overflow (DoS on untrusted input, same class as the5BAApanic fixed in #74); in release it wraps to a small bogusfs, and the subsequentstream.len() < fscheck then passes, slicing a truncated frame that parses as valid (silently-wrong parse — exactly what the rule warns about). The adjacent:276already used.checked_add, so these were inconsistent oversights.Fix
Extract the three sites into checked helpers in
builder.rs:compute_full_size(size, cs)→checked_mul(4)+checked_add(cs)compute_qb2_byte_size(fs)→checked_mul(3)thendiv_ceil(4)Both return a new typed
ValidationError::SizeOverflowon overflow (distinct failure domain fromIncorrectRawSize). Call sites propagate via?.Reachability (resolves the issue's open question)
Latent, not reachable through the parse API today. Variable-size codes cap the soft field at
ss=4(SizeType::Large), sosize ≤ 64⁴−1 = 2²⁴−1; thensize*4 ≈ 2²⁶fits even 32-bitusize(wasm32). Reaching overflow needssize ≥ 2³⁰(a ≥5-char soft field no current code has). Fixed as a Mandatory-Rule violation + defense-in-depth for any future wider soft field.Because the overflow is unreachable via
from_qualified_base64/base2, there is no fuzz-corpus seed to add — the tests probe the checked helpers directly instead.Tests (TDD)
checked_*, all 5 pass (SizeOverflowreturned), plus 61/61 builder+error tests and the fullnix flake checkgate.Breaking change
ValidationErrorgains aSizeOverflowvariant. As the enum is public and not#[non_exhaustive], downstream exhaustivematchmust handle it — MINOR under the 0.x SemVer convention. Noted inCHANGELOG.Verification
nix flake check— all checks green (clippy, fmt, taplo, audit, deny, nextest, doctest, wasm, no_std, fuzz-replay).🤖 Generated with Claude Code