wasmtime / issue #5344 Unexpected behavior · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #5344 Unexpected behavior

Wasmtime GitHub notifications bot (Nov 29 2022 at 17:14):

erxiaozhou opened issue #5344:

Environment

Code Version: git commit id ff5abfd9938c78cd75c6c5d6a41565097128c5d3
Hardware Architecture: x86_64
Operating system: Ubuntu 20.04

reply_570_2_unexpected_execution.zip

command
target/release/wasmtime --invoke to_test reply_wasmi_570_2/replay_570_2_unexpected_execution.wasm
Expected behavior:

An exception indicating that "alignment must not be larger than natural"

Current behavior:
Output: 1
Steps to generate this test case

570_2.zip

Wasmtime GitHub notifications bot (Nov 29 2022 at 17:29):

alexcrichton commented on issue #5344:

Thanks for the report, but this is a somewhat benign issue. The problem I think you're running into here is that the provided module is encoded as:
...
 0x44 | 37 40 00 44 | i64_store memarg:MemArg { align: 0, max_align: 3, offset: 68, memory: 0 }
...
where the i64.store is encoded with 0x37 0x40 0x00 0x44. Using an MVP decoder that decodes as an alignment of 0x40 and an offset of 0x00 which is indeed, as you've pointed out, an error as "alignment must not be larger than natural". The wasmparser crate which Wasmtime uses, however, implements the multi-memory proposal which reinterprets the binary encoding of a memarg where 0x40 means "alignment is 1 and the next byte is memory" which causes wasmparser to decode to the above structure, which is indeed valid (under the multi-memory proposal).

This more-or-less a bug in wasmparser's binary decoding and/or validation. Right now features are not checked/verified at the binary decoding layer, only at the AST structural layer, meaning that this binary encoding is not properly gated or conditionalized behind the multi_memory proposal at this time.

Would you be interested in helping to fix this?

Wasmtime GitHub notifications bot (Dec 01 2022 at 08:00):

erxiaozhou commented on issue #5344:

Thanks for the report, but this is a somewhat benign issue. The problem I think you're running into here is that the provided module is encoded as:

... 0x44 | 37 40 00 44 | i64_store memarg:MemArg { align: 0, max_align: 3, offset: 68, memory: 0 } ...

where the i64.store is encoded with 0x37 0x40 0x00 0x44. Using an MVP decoder that decodes as an alignment of 0x40 and an offset of 0x00 which is indeed, as you've pointed out, an error as "alignment must not be larger than natural". The wasmparser crate which Wasmtime uses, however, implements the multi-memory proposal which reinterprets the binary encoding of a memarg where 0x40 means "alignment is 1 and the next byte is memory" which causes wasmparser to decode to the above structure, which is indeed valid (under the multi-memory proposal).

This more-or-less a bug in wasmparser's binary decoding and/or validation. Right now features are not checked/verified at the binary decoding layer, only at the AST structural layer, meaning that this binary encoding is not properly gated or conditionalized behind the multi_memory proposal at this time.

Would you be interested in helping to fix this?

Thank you for your nice reply! I'll be happy to fix this after I finish my current work.

Wasmtime GitHub notifications bot (Oct 06 2024 at 21:56):

Robbepop commented on issue #5344:

@alexcrichton has this been fixed already with BinaryReader including WasmFeatures now since some versions of wasmparser? https://docs.rs/wasmparser/0.218.0/wasmparser/struct.BinaryReader.html#method.new_features

Wasmtime GitHub notifications bot (Oct 06 2024 at 22:10):

alexcrichton closed issue #5344:

Environment

Code Version: git commit id ff5abfd9938c78cd75c6c5d6a41565097128c5d3
Hardware Architecture: x86_64
Operating system: Ubuntu 20.04

reply_570_2_unexpected_execution.zip

command
target/release/wasmtime --invoke to_test reply_wasmi_570_2/replay_570_2_unexpected_execution.wasm
Expected behavior:

An exception indicating that "alignment must not be larger than natural"

Current behavior:
Output: 1
Steps to generate this test case

570_2.zip

Wasmtime GitHub notifications bot (Oct 06 2024 at 22:10):

alexcrichton commented on issue #5344:

Good point! This now reports:
$ wasmtime run -W multi-memory=n reply_570_2_unexpected_execution.wasm
Error: WebAssembly translation error

Caused by:
    Invalid input WebAssembly code at offset 69: malformed memop alignment: alignment too large
which I believe is the correct error here. Specifically this module is valid-by-default due to the multi-memory proposal, but it's invalid if the multi-memory proposal is disabled.

Last updated: Nov 22 2024 at 16:03 UTC