From b9e7f624f2ddfffb77baf561c8eca60a5b8b1d8f Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 3 Apr 2023 13:02:40 +0200 Subject: [PATCH 1/5] Disable post-MVP Wasm features --- lib/src/executor/vm/interpreter.rs | 17 ++- lib/src/executor/vm/jit.rs | 12 ++ lib/src/executor/vm/tests.rs | 181 +++++++++++++++++++++++++++++ wasm-node/CHANGELOG.md | 4 + 4 files changed, 213 insertions(+), 1 deletion(-) diff --git a/lib/src/executor/vm/interpreter.rs b/lib/src/executor/vm/interpreter.rs index 276a171b1b..2a84c3955a 100644 --- a/lib/src/executor/vm/interpreter.rs +++ b/lib/src/executor/vm/interpreter.rs @@ -54,7 +54,22 @@ impl InterpreterPrototype { module_bytes: &[u8], symbols: &mut dyn FnMut(&str, &str, &Signature) -> Result, ) -> Result { - let engine = wasmi::Engine::default(); // TODO: investigate config + let engine = { + let mut config = wasmi::Config::default(); + + // Disable all the post-MVP wasm features. + config.wasm_sign_extension(false); + config.wasm_reference_types(false); + config.wasm_bulk_memory(false); + config.wasm_multi_value(false); + config.wasm_extended_const(false); + config.wasm_mutable_global(false); + config.wasm_saturating_float_to_int(false); + config.wasm_tail_call(false); + + wasmi::Engine::new(&config) + }; + let module = wasmi::Module::new(&engine, module_bytes) .map_err(|err| NewErr::InvalidWasm(err.to_string()))?; diff --git a/lib/src/executor/vm/jit.rs b/lib/src/executor/vm/jit.rs index 38e8497095..62215a4423 100644 --- a/lib/src/executor/vm/jit.rs +++ b/lib/src/executor/vm/jit.rs @@ -78,6 +78,18 @@ impl JitPrototype { // environment variables whatsoever. Whether to use `Enable` or `Disable` below isn't // very important, so long as it is not `Environment`. config.wasm_backtrace_details(wasmtime::WasmBacktraceDetails::Enable); + + // Disable all post-MVP wasm features. + // Some of these configuration options are `true` by default while some others are `false` + // by default, but we just disable them all to be sure. + config.wasm_threads(false); + config.wasm_reference_types(false); + config.wasm_simd(false); + config.wasm_bulk_memory(false); + config.wasm_multi_value(false); + config.wasm_multi_memory(false); + config.wasm_memory64(false); + let engine = wasmtime::Engine::new(&config).map_err(|err| NewErr::InvalidWasm(err.to_string()))?; diff --git a/lib/src/executor/vm/tests.rs b/lib/src/executor/vm/tests.rs index 615d5ced43..5ea2f199e2 100644 --- a/lib/src/executor/vm/tests.rs +++ b/lib/src/executor/vm/tests.rs @@ -841,4 +841,185 @@ fn memory_zeroed_after_prepare() { } } +#[test] +fn feature_disabled_signext() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (result i64) + (i64.const 2) + i64.extend32_s + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + // TODO: wasmtime doesn't allow disabling sign-ext /!\ test is faulty /!\ figure out what to do + // TODO: see https://github.com/paritytech/substrate/issues/10707#issuecomment-1494081313 + if Some(exec_hint) == super::ExecHint::force_wasmtime_if_available() { + continue; + } + + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_saturated_float_to_int() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (result i64) + (f32.const 2) + i64.trunc_sat_f32_s + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + // TODO: wasmtime doesn't allow disabling this feature /!\ test is faulty /!\ figure out what to do + // TODO: see https://github.com/paritytech/substrate/issues/10707#issuecomment-1494081313 + if Some(exec_hint) == super::ExecHint::force_wasmtime_if_available() { + continue; + } + + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_threads() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (result i64) + (atomic.fence) + i64.const 2 + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_reference_type() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (result externref) unreachable) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_bulk_memory() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (result i64) + (memory.fill (i32.const 0) (i32.const 0) (i32.const 0)) + i64.const 2 + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_multi_value() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func (export "hello") (param i64 i64) (result i64 i64 i64) + (local.get 0) (local.get 1) (local.get 0) + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +#[test] +fn feature_disabled_memory64() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem i64 0 4096)) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +// TODO: several wasm feature checks are missing + // TODO: test for memory reads and writes, including within host functions diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index f33f310653..345b6587bc 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -6,6 +6,10 @@ - Removed support for the `ls` message in the multistream-select protocol, in accordance with the rest of the libp2p ecosystem. This message was in practice never used, and removing support for it simplifies the implementation. ([#379](https://github.com/smol-dot/smoldot/pull/379)) +### Fixed + +- Post-MVP WebAssembly features are now properly disabled when compiling runtimes. This rejects runtimes that Substrate would consider as invalid as well. + ## 1.0.1 - 2023-03-29 ### Changed From c2a6e3e4b3b047eb467aaa1fac260982753b1315 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 3 Apr 2023 13:03:35 +0200 Subject: [PATCH 2/5] PR number --- wasm-node/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wasm-node/CHANGELOG.md b/wasm-node/CHANGELOG.md index 345b6587bc..cfbdc650d8 100644 --- a/wasm-node/CHANGELOG.md +++ b/wasm-node/CHANGELOG.md @@ -8,7 +8,7 @@ ### Fixed -- Post-MVP WebAssembly features are now properly disabled when compiling runtimes. This rejects runtimes that Substrate would consider as invalid as well. +- Post-MVP WebAssembly features are now properly disabled when compiling runtimes. This rejects runtimes that Substrate would consider as invalid as well. ([#386](https://github.com/smol-dot/smoldot/pull/386)) ## 1.0.1 - 2023-03-29 From 01a1e3921334909f0086999500c7b4aaa13d9d26 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 3 Apr 2023 17:03:21 +0200 Subject: [PATCH 3/5] Adjust tests and add one for `mutable-globals` --- lib/src/executor/vm/tests.rs | 37 ++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/src/executor/vm/tests.rs b/lib/src/executor/vm/tests.rs index 5ea2f199e2..e401ef6251 100644 --- a/lib/src/executor/vm/tests.rs +++ b/lib/src/executor/vm/tests.rs @@ -308,7 +308,7 @@ fn unsupported_signature() { let module_bytes = wat::parse_str( r#" (module - (import "host" "hello" (func $host_hello (param i32) (param externref) (result i32))) + (import "host" "hello" (func $host_hello (param i32) (param f64) (result i32))) (import "env" "memory" (memory $mem 0 4096)) (func (export "hello") (result i32) i32.const 0) ) @@ -460,7 +460,7 @@ fn call_signature_not_supported() { r#" (module (import "env" "memory" (memory $mem 0 4096)) - (func (export "hello") (result externref) unreachable) + (func (export "hello") (result f64) unreachable) ) "#, ) @@ -725,7 +725,8 @@ fn memory_grow_detects_limit_within_host_function() { } } -#[test] +// TODO: re-enable this test if the `mutable-globals` feature is enabled +/*#[test] fn globals_reinitialized_after_reset() { let module_bytes = wat::parse_str( r#" @@ -762,7 +763,7 @@ fn globals_reinitialized_after_reset() { let mut prototype = vm.into_prototype(); assert_eq!(prototype.global_value("myglob").unwrap(), 5); } -} +}*/ #[test] fn memory_zeroed_after_reset() { @@ -1020,6 +1021,34 @@ fn feature_disabled_memory64() { } } +#[test] +fn feature_disabled_mutable_globals() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (global $myglob (export "myglob") (mut i32) (i32.const 5)) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + // TODO: wasmtime doesn't allow disabling this feature /!\ test is faulty /!\ figure out what to do + // TODO: see https://github.com/paritytech/substrate/issues/10707#issuecomment-1494081313 + if Some(exec_hint) == super::ExecHint::force_wasmtime_if_available() { + continue; + } + + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + // TODO: several wasm feature checks are missing // TODO: test for memory reads and writes, including within host functions From b5f535a8917d6ce767623c374bc10299d2bca656 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 3 Apr 2023 17:09:23 +0200 Subject: [PATCH 4/5] Check tail-call feature --- lib/src/executor/vm/tests.rs | 38 +++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/src/executor/vm/tests.rs b/lib/src/executor/vm/tests.rs index e401ef6251..d7f9dc26d7 100644 --- a/lib/src/executor/vm/tests.rs +++ b/lib/src/executor/vm/tests.rs @@ -1049,6 +1049,42 @@ fn feature_disabled_mutable_globals() { } } -// TODO: several wasm feature checks are missing +#[test] +fn feature_disabled_tail_call() { + let module_bytes = wat::parse_str( + r#" + (module + (import "env" "memory" (memory $mem 0 4096)) + (func $fac (param $x i64) (result i64) + (return_call $fac-aux (get_local $x) (i64.const 1)) + ) + (func $fac-aux (param $x i64) (param $r i64) (result i64) + (if (i64.eqz (get_local $x)) + (then (return (get_local $r))) + (else + (return_call $fac-aux + (i64.sub (get_local $x) (i64.const 1)) + (i64.mul (get_local $x) (get_local $r)) + ) + ) + ) + ) + ) + "#, + ) + .unwrap(); + + for exec_hint in super::ExecHint::available_engines() { + assert!(super::VirtualMachinePrototype::new(super::Config { + module_bytes: &module_bytes, + exec_hint, + symbols: &mut |_, _, _| Ok(0), + }) + .is_err()); + } +} + +// TODO: check that the SIMD feature is disabled: https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md +// TODO: check that the extended-const feature is disabled: https://github.com/WebAssembly/extended-const/blob/master/proposals/extended-const/Overview.md // TODO: test for memory reads and writes, including within host functions From 99e9c0a7d1ce682a68cc5ffe106a522ffd097cce Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 3 Apr 2023 17:40:26 +0200 Subject: [PATCH 5/5] Document the features --- lib/src/executor/vm.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/src/executor/vm.rs b/lib/src/executor/vm.rs index b8abd543a6..7dd245d778 100644 --- a/lib/src/executor/vm.rs +++ b/lib/src/executor/vm.rs @@ -52,6 +52,16 @@ //! //! The first variant used to be the default model when compiling to WebAssembly, but the second //! variant (importing memory objects) is preferred nowadays. +//! +//! # Wasm features +//! +//! The WebAssembly specification is a moving one. The specification as it was when it launched +//! in 2017 is commonly referred to as "the MVP" (minimum viable product). Since then, various +//! extensions have been added to the WebAssembly format. +//! +//! The code in this module, however, doesn't allow any of the feature that were added post-MVP. +//! Trying to use WebAssembly code that uses one of these features will result in an error. +//! mod interpreter; #[cfg(all(target_arch = "x86_64", feature = "std"))]