diff --git a/Cargo.lock b/Cargo.lock index dbb046ccd86af..348f23d736d37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8270,6 +8270,7 @@ name = "sc-executor" version = "0.10.0-dev" dependencies = [ "array-bytes", + "assert_matches", "criterion", "env_logger", "lru", @@ -8842,6 +8843,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "sp-runtime-interface", "sp-std", "substrate-wasm-builder", ] diff --git a/client/executor/Cargo.toml b/client/executor/Cargo.toml index 4604b976b4248..8bedcd3edcff3 100644 --- a/client/executor/Cargo.toml +++ b/client/executor/Cargo.toml @@ -35,6 +35,7 @@ sp-wasm-interface = { version = "7.0.0", path = "../../primitives/wasm-interface [dev-dependencies] array-bytes = "4.1" +assert_matches = "1.3.0" wat = "1.0" sc-runtime-test = { version = "2.0.0", path = "runtime-test" } substrate-test-runtime = { version = "2.0.0", path = "../../test-utils/runtime" } diff --git a/client/executor/runtime-test/Cargo.toml b/client/executor/runtime-test/Cargo.toml index e99f3caa9447e..07626c2343361 100644 --- a/client/executor/runtime-test/Cargo.toml +++ b/client/executor/runtime-test/Cargo.toml @@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"] sp-core = { version = "7.0.0", default-features = false, path = "../../../primitives/core" } sp-io = { version = "7.0.0", default-features = false, features = ["improved_panic_error_reporting"], path = "../../../primitives/io" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../../primitives/runtime" } +sp-runtime-interface = { version = "7.0.0", default-features = false, path = "../../../primitives/runtime-interface" } sp-std = { version = "5.0.0", default-features = false, path = "../../../primitives/std" } [build-dependencies] diff --git a/client/executor/runtime-test/src/lib.rs b/client/executor/runtime-test/src/lib.rs index fc98d1909d00d..0aa30e4bc9675 100644 --- a/client/executor/runtime-test/src/lib.rs +++ b/client/executor/runtime-test/src/lib.rs @@ -29,6 +29,8 @@ use sp_runtime::{ print, traits::{BlakeTwo256, Hash}, }; +#[cfg(not(feature = "std"))] +use sp_runtime_interface::pack_ptr_and_len; extern "C" { #[allow(dead_code)] @@ -38,6 +40,10 @@ extern "C" { fn yet_another_missing_external(); } +#[cfg(not(feature = "std"))] +/// The size of a WASM page in bytes. +const WASM_PAGE_SIZE: usize = 65536; + #[cfg(not(feature = "std"))] /// Mutable static variables should be always observed to have /// the initialized value at the start of a runtime call. @@ -92,7 +98,7 @@ sp_core::wasm_export_functions! { let heap_ptr = heap_base as usize; // Find the next wasm page boundary. - let heap_ptr = round_up_to(heap_ptr, 65536); + let heap_ptr = round_up_to(heap_ptr, WASM_PAGE_SIZE); // Make it an actual pointer let heap_ptr = heap_ptr as *mut u8; @@ -337,3 +343,32 @@ sp_core::wasm_export_functions! { return 1234; } } + +// Returns a huge len. It should result in an error, and not an allocation. +#[no_mangle] +#[cfg(not(feature = "std"))] +pub extern "C" fn test_return_huge_len(_params: *const u8, _len: usize) -> u64 { + pack_ptr_and_len(0, u32::MAX) +} + +// Returns an offset right at the edge of the wasm memory boundary. With length 0, it should +// succeed. +#[no_mangle] +#[cfg(not(feature = "std"))] +pub extern "C" fn test_return_max_memory_offset(_params: *const u8, _len: usize) -> u64 { + pack_ptr_and_len((core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32, 0) +} + +// Returns an offset right at the edge of the wasm memory boundary. With length 1, it should fail. +#[no_mangle] +#[cfg(not(feature = "std"))] +pub extern "C" fn test_return_max_memory_offset_plus_one(_params: *const u8, _len: usize) -> u64 { + pack_ptr_and_len((core::arch::wasm32::memory_size(0) * WASM_PAGE_SIZE) as u32, 1) +} + +// Returns an output that overflows the u32 range. It should result in an error. +#[no_mangle] +#[cfg(not(feature = "std"))] +pub extern "C" fn test_return_overflow(_params: *const u8, _len: usize) -> u64 { + pack_ptr_and_len(u32::MAX, 1) +} diff --git a/client/executor/src/integration_tests/mod.rs b/client/executor/src/integration_tests/mod.rs index 25b999f115363..22e651abe98e2 100644 --- a/client/executor/src/integration_tests/mod.rs +++ b/client/executor/src/integration_tests/mod.rs @@ -19,8 +19,13 @@ #[cfg(target_os = "linux")] mod linux; +use assert_matches::assert_matches; use codec::{Decode, Encode}; -use sc_executor_common::{error::Error, runtime_blob::RuntimeBlob, wasm_runtime::WasmModule}; +use sc_executor_common::{ + error::{Error, WasmError}, + runtime_blob::RuntimeBlob, + wasm_runtime::WasmModule, +}; use sc_runtime_test::wasm_binary_unwrap; use sp_core::{ blake2_128, blake2_256, ed25519, map, @@ -781,3 +786,67 @@ fn return_value(wasm_method: WasmExecutionMethod) { (1234u64).encode() ); } + +test_wasm_execution!(return_huge_len); +fn return_huge_len(wasm_method: WasmExecutionMethod) { + let mut ext = TestExternalities::default(); + let mut ext = ext.ext(); + + match call_in_wasm("test_return_huge_len", &[], wasm_method, &mut ext).unwrap_err() { + Error::Runtime => { + assert_matches!(wasm_method, WasmExecutionMethod::Interpreted); + }, + Error::RuntimeConstruction(WasmError::Other(error)) => { + assert_matches!(wasm_method, WasmExecutionMethod::Compiled { .. }); + assert_eq!(error, "output exceeds bounds of wasm memory"); + }, + error => panic!("unexpected error: {:?}", error), + } +} + +test_wasm_execution!(return_max_memory_offset); +fn return_max_memory_offset(wasm_method: WasmExecutionMethod) { + let mut ext = TestExternalities::default(); + let mut ext = ext.ext(); + + assert_eq!( + call_in_wasm("test_return_max_memory_offset", &[], wasm_method, &mut ext).unwrap(), + ().encode() + ); +} + +test_wasm_execution!(return_max_memory_offset_plus_one); +fn return_max_memory_offset_plus_one(wasm_method: WasmExecutionMethod) { + let mut ext = TestExternalities::default(); + let mut ext = ext.ext(); + + match call_in_wasm("test_return_max_memory_offset_plus_one", &[], wasm_method, &mut ext) + .unwrap_err() + { + Error::Runtime => { + assert_matches!(wasm_method, WasmExecutionMethod::Interpreted); + }, + Error::RuntimeConstruction(WasmError::Other(error)) => { + assert_matches!(wasm_method, WasmExecutionMethod::Compiled { .. }); + assert_eq!(error, "output exceeds bounds of wasm memory"); + }, + error => panic!("unexpected error: {:?}", error), + } +} + +test_wasm_execution!(return_overflow); +fn return_overflow(wasm_method: WasmExecutionMethod) { + let mut ext = TestExternalities::default(); + let mut ext = ext.ext(); + + match call_in_wasm("test_return_overflow", &[], wasm_method, &mut ext).unwrap_err() { + Error::Runtime => { + assert_matches!(wasm_method, WasmExecutionMethod::Interpreted); + }, + Error::RuntimeConstruction(WasmError::Other(error)) => { + assert_matches!(wasm_method, WasmExecutionMethod::Compiled { .. }); + assert_eq!(error, "output exceeds bounds of wasm memory"); + }, + error => panic!("unexpected error: {:?}", error), + } +} diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index b124fd627dc69..a45dfa59b954a 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -30,6 +30,7 @@ use sc_executor_common::{ runtime_blob::{ self, DataSegmentsSnapshot, ExposedMutableGlobalsSet, GlobalsSnapshot, RuntimeBlob, }, + util::checked_range, wasm_runtime::{InvokeMethod, WasmInstance, WasmModule}, }; use sp_runtime_interface::unpack_ptr_and_len; @@ -41,7 +42,7 @@ use std::{ Arc, }, }; -use wasmtime::{Engine, Memory, StoreLimits, Table}; +use wasmtime::{AsContext, Engine, Memory, StoreLimits, Table}; pub(crate) struct StoreData { /// The limits we apply to the store. We need to store it here to return a reference to this @@ -793,7 +794,19 @@ fn extract_output_data( output_ptr: u32, output_len: u32, ) -> Result> { + let ctx = instance.store(); + + // Do a length check before allocating. The returned output should not be bigger than the + // available WASM memory. Otherwise, a malicious parachain can trigger a large allocation, + // potentially causing memory exhaustion. + // + // Get the size of the WASM memory in bytes. + let memory_size = ctx.as_context().data().memory().data_size(ctx); + if checked_range(output_ptr as usize, output_len as usize, memory_size).is_none() { + Err(WasmError::Other("output exceeds bounds of wasm memory".into()))? + } let mut output = vec![0; output_len as usize]; - util::read_memory_into(instance.store(), Pointer::new(output_ptr), &mut output)?; + + util::read_memory_into(ctx, Pointer::new(output_ptr), &mut output)?; Ok(output) }