Repository navigation
fix(#953): rv32 (memory 0) must trap every access, not invent a 64 KiB bound (SECURITY) - #955
Merged
Merged
Conversation
…KiB bound (SECURITY) A module declaring `(memory 0)` got a bound baked at 65532 — byte-for-byte the guard of a 1-page module — so every address in 0..=65532 passed the `--safety-bounds software` check and the access was performed against a memory with no bytes. Reads AND writes. Reported by gale (#953) 30 minutes after v0.56.0 published; reproduced on the shipped tag. ## Root cause: `0` as both a value and a sentinel let mem_size = if config.linear_memory_bytes > 0 { .. } else { 64 * 1024 }; The fallback's stated purpose is real — a hand-built driver with no module context still needs a bound. But `0` is ALSO exactly what `(memory 0)` produces, and the two are indistinguishable, so it invented a bound for a module that declared none. **Same disease as #932, fixed one release earlier, running the other way.** There `unwrap_or(0)` turned "no floor establishable" into "the floor is 0 bytes" and STRIPPED guards; here `0` meant "unset" and INVENTED one. ## Why deleting the fallback is the fix, not a workaround Measured rather than assumed. With it removed, exactly ONE test in the tree failed — and the bytes it rejected contained `0x00100073` (`ebreak`): rv32 already folds to an unconditional trap at size 0, which is the correct answer and precisely what aarch64 does (`brk #0`). The other two backends were right all along: aarch64 reads the value raw, ARM derives its bound from a runtime register with an underflow check. rv32 was the anomaly. So the contract is now: `linear_memory_bytes` is the size, full stop; 0 means zero bytes. A caller with no module context states the size — which is what a real driver does, and what the one affected test now does. Leaving it 0 fails CLOSED (everything traps) rather than open. ## Evidence — red-first, executed against wasmtime `scripts/repro/rv32_zero_memory_953_differential.py`, CI-wired. 9 vectors across load/store/load8, each asserting synth TRAPS iff wasmtime traps — a trap is observed as a real `ebreak` intercept, not as "the value came back wrong", which would pass vacuously if the function returned 0 for an unrelated reason. pre-fix 6/9 FAIL (ld/st/ld8 at 0 and at 65528 all performed the access) post-fix 9/9 PASS The three 65536 vectors are the NON-VACUITY control: they trapped before the fix too, so the fixture is demonstrably not just trapping on everything. synth-backend-riscv 259 passed / 0 failed; frozen anchors 10/10; clippy clean. Emulation-floor ledger 295701 -> 295710 in both surfaces. The instance is fixed here. The MECHANISM — every numeric sentinel that means both a value and its own absence — is RQ-57-SENTINEL, and it must produce a count of sites examined and converted or it has not been done. Closes #953 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Reported by gale 30 minutes after v0.56.0 published; reproduced on the shipped tag.
A module declaring
(memory 0)got a bound baked at 65532 — byte-for-bytethe guard of a 1-page module — so every address in
0..=65532passed the--safety-bounds softwarecheck and the access was performed against a memorywith no bytes. Reads and writes.
Root cause:
0as both a value and a sentinelThe fallback's stated purpose is real — a hand-built driver with no module
context still needs a bound. But
0is also exactly what(memory 0)produces, and the two are indistinguishable, so it invented a bound for a module
that declared none.
This is the same disease as #932 — fixed one release earlier — running the
other way. There
unwrap_or(0)turned "no floor establishable" into "the flooris 0 bytes" and stripped guards; here
0meant "unset" and invented one.Why deleting the fallback is the fix, not a workaround
Measured, not assumed. With it removed, exactly one test in the tree failed —
and the bytes it rejected contained
0x00100073(ebreak). rv32 already foldsto an unconditional trap at size 0, which is the correct answer and precisely
what aarch64 does (
brk #0).The other two backends were right all along: aarch64 reads the value raw, ARM
derives its bound from a runtime register with an underflow check. rv32 was the
anomaly — which is what makes this a fallback to delete rather than a design
gap to fill.
The contract is now:
linear_memory_bytesis the size;0means zero bytes. Acaller with no module context states the size — what a real driver does, and what
the one affected test now does. Leaving it
0fails closed.Evidence — red-first, executed against wasmtime
scripts/repro/rv32_zero_memory_953_differential.py, CI-wired. 9 vectors acrossload / store / load8, each asserting synth traps iff wasmtime traps.
ld/st/ld8at 0 and at 65528 all performed the accessA trap is observed as a real
ebreakintercept, not as "the value came backwrong" — a value check would pass vacuously if the function returned 0 for an
unrelated reason. The three
65536vectors are the non-vacuity control: theytrapped before the fix too, so the fixture demonstrably isn't just trapping on
everything.
synth-backend-riscv259 passed / 0 failed · frozen anchors 10/10 ·clippy clean · emulation-floor ledger 295701 → 295710 in both surfaces.
The instance is fixed here; the mechanism is not
RQ-57-SENTINEL(in the v0.57 plan, #954) is the sweep for every numericsentinel that means both a value and its own absence — and it must produce a
count of sites examined and converted, or it hasn't been done. Two of these have
now shipped one release apart, both SECURITY.
Closes #953
🤖 Generated with Claude Code
https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L