Fix potential huge allocation as a result of validate_block output

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" }

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]

client/executor/runtime-test/src/lib.rs

@@ -30,6 +30,8 @@ use sp_runtime::{
 	traits::{BlakeTwo256, Hash},
 };
 
+use sp_runtime_interface::pack_ptr_and_len;
+
 extern "C" {
 	#[allow(dead_code)]
 	fn missing_external();
@@ -337,3 +339,15 @@ sp_core::wasm_export_functions! {
 		return 1234;
 	}
 }
+
+// Returns a huge len. It should result in an error, and not an allocation.
+#[no_mangle]
+pub extern "C" fn test_return_huge_len(_params: *const u8, _len: usize) -> u64 {
+	pack_ptr_and_len(0, u32::MAX)
+}
+
+// Returns an output that overflows the u32 range. It should result in an error.
+#[no_mangle]
+pub extern "C" fn test_return_overflow(_params: *const u8, _len: usize) -> u64 {
+	pack_ptr_and_len(u32::MAX, 1)
+}

client/executor/src/integration_tests/mod.rs

@@ -20,7 +20,7 @@
 mod linux;
 
 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,
@@ -33,6 +33,7 @@ use sp_runtime::traits::BlakeTwo256;
 use sp_state_machine::TestExternalities as CoreTestExternalities;
 use sp_trie::{LayoutV1 as Layout, TrieConfiguration};
 use std::sync::Arc;
+use assert_matches::assert_matches;
 use tracing_subscriber::layer::SubscriberExt;
 
 use crate::WasmExecutionMethod;
@@ -781,3 +782,37 @@ 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_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),
+	}
+}

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::{Engine, Memory, StoreLimits, Table, AsContext};
 
 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<Vec<u8>> {
+	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 let None = checked_range(output_ptr as usize, output_len as usize, memory_size) {
+		return 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)
 }