fix(compiler): disable unimplemented wasm proposals in the validator and reject their opcodes - #173
Open
dmitry123 wants to merge 1 commit into
Open
fix(compiler): disable unimplemented wasm proposals in the validator and reject their opcodes#173dmitry123 wants to merge 1 commit into
dmitry123 wants to merge 1 commit into
Conversation
…and reject their opcodes
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe compiler now enumerates WebAssembly feature flags explicitly, rejects unsupported operators with ChangesWasm feature enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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-1095 (CRIT-2).
Problem
CompilationConfig::wasm_featuresenumerated the supported proposals and filled the rest with..Default::default(). Inwasmparser-nostd 0.100.2,simdis on by default, so the validator acceptedv128instructions that the translator does not implement.impl_visit_operator!routes every unlisted proposal to a wildcard arm that validated the operator and translated nothing. The validator's operand stack advanced; no opcode was emitted and neitherstack_heightnorstack_typeswas updated. From theredrop_keepamounts andlocal.get/local.setdepths were computed against a stack height that did not describe the emitted code.Confirmed today as a compiler panic — a DoS on the deployment path, reachable from untrusted wasm:
The latent case is worse: a shape that misses that assert yields a wrong
DropKeepand wrong local depths in a module that otherwise looks valid, and per CRIT-1 the interpreter does not bounds-check those depths.Fix
Two independent gates.
1.
src/compiler/config.rs— the validator no longer accepts more than the translator implements. EveryWasmFeaturesfield is now listed explicitly;..Default::default()is gone.simd,relaxed_simd,threads,multi_memory,memory64,exceptions,component_modelandmemory_controlare set tofalse.floatsstaystrue(floats are translated).Spelling out every field also makes the failure mode of a
wasmparserupgrade the right one: a bump that adds a field now fails to compile instead of inheriting an unreviewed default, and a bump that flips one fails the test below.2.
src/compiler/func_builder.rs— the wildcard arm returnsErr(CompilationError::NotSupportedOpcode)instead of validating-and-ignoring. Gate 1 already rejects every proposal that reaches this arm, so this is defence in depth: it is what stops the two lists drifting apart again, since today the only thing preventing a silent skip is that the validator happens to reject first.NotSupportedOpcodeis a newCompilationErrorvariant.On the issue's point 4 —
translate_localsneeded no change. It callsvalidator.define_localsbefore its ownValType::V128 => NotSupportedLocalTypecheck, so gate 1 now rejects av128local first ("SIMD support is not enabled"); the existing arm stays as a guard. The operand path now mirrors it.Tests
New
tests/wasm_features.rs:NotSupportedOpcodeinstead of panicking.component_modelandmemory_control, which no core module can express in WAT. This is the test that catches awasmparserdefault flip.Note that relaxed-SIMD has no shape that reaches its own flag: every relaxed operator takes or returns a
v128, so the SIMD gate is what rejects it. The test documents this rather than pretending otherwise.Verification
cargo test— full suite green.cargo testine2e/— 92/92 spec tests pass. The spec list contains no SIMD entries, so nothing there regressed.cargo clippy --all-targetsclean;cargo fmt --checkclean for every file touched here.Summary by CodeRabbit
Bug Fixes
Tests