From 31b304a97abb61b3a899541006a5193f820c0f56 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 19 Dec 2023 10:21:59 +0000 Subject: [PATCH 1/3] Sanitize tx and vp wasm memory accesses --- shared/src/vm/wasm/memory.rs | 38 +++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/shared/src/vm/wasm/memory.rs b/shared/src/vm/wasm/memory.rs index b93ecb79f19..f8b5c554bb0 100644 --- a/shared/src/vm/wasm/memory.rs +++ b/shared/src/vm/wasm/memory.rs @@ -23,6 +23,8 @@ use crate::vm::types::VpInput; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { + #[error("Offset {0}+{1} overflows 32 bits storage")] + OverflowingOffset(u64, usize), #[error("Failed initializing the memory: {0}")] InitMemoryError(wasmer::MemoryError), #[error("Memory ouf of bounds: {0}")] @@ -167,22 +169,44 @@ pub fn write_vp_inputs( /// Check that the given offset and length fits into the memory bounds. If not, /// it will try to grow the memory. -fn check_bounds(memory: &Memory, offset: u64, len: usize) -> Result<()> { +fn check_bounds(memory: &Memory, base_offset: u64, len: usize) -> Result<()> { tracing::debug!( - "check_bounds pages {}, data_size {}, offset + len {}", + "check_bounds pages {}, data_size {}, base_offset + len {}", memory.size().0, memory.data_size(), - offset + len as u64 + base_offset + len as u64 ); - if memory.data_size() < offset + len as u64 { + let desired_offset = base_offset + .checked_add(len as u64) + .and_then(|off| { + if off < u32::MAX as u64 { + // wasm pointers are 32 bits wide, therefore we can't + // read from/write to offsets past `u32::MAX` + Some(off) + } else { + None + } + }) + .ok_or(Error::OverflowingOffset(base_offset, len))?; + if memory.data_size() < desired_offset { let cur_pages = memory.size().0; let capacity = cur_pages as usize * wasmer::WASM_PAGE_SIZE; - let missing = offset as usize + len - capacity; - // Ceiling division + // usizes should be at least 32 bits wide on most architectures, + // so this cast shouldn't cause panics, given the invariant that + // `desired_offset` is at most a 32 bit wide value. moreover, + // `capacity` should not be larger than `memory.data_size()`, + // so this subtraction should never fail + let missing = desired_offset as usize - capacity; + // extrapolate the number of pages missing to allow addressing + // the desired memory offset let req_pages = ((missing + wasmer::WASM_PAGE_SIZE - 1) / wasmer::WASM_PAGE_SIZE) as u32; - tracing::info!("trying to grow memory by {} pages", req_pages); + tracing::debug!(req_pages, "Attempting to grow wasm memory"); memory.grow(req_pages).map_err(Error::MemoryOutOfBounds)?; + tracing::debug!( + mem_size = memory.data_size(), + "Wasm memory size has been successfully extended" + ); } Ok(()) } From 795129bc468155de891b523028b23316c543a7f5 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Wed, 20 Dec 2023 14:27:23 +0000 Subject: [PATCH 2/3] Add wasm ptr overflow checks regression test --- shared/src/vm/wasm/run.rs | 126 ++++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 31 deletions(-) diff --git a/shared/src/vm/wasm/run.rs b/shared/src/vm/wasm/run.rs index 7b7a009c6a1..3c04e56ce3f 100644 --- a/shared/src/vm/wasm/run.rs +++ b/shared/src/vm/wasm/run.rs @@ -630,6 +630,8 @@ fn get_gas_rules() -> wasm_instrument::gas_metering::ConstantCostRules { #[cfg(test)] mod tests { + use std::error::Error as StdErrorTrait; + use borsh_ext::BorshSerializeExt; use itertools::Either; use namada_test_utils::TestWasms; @@ -642,10 +644,68 @@ mod tests { use crate::types::hash::Hash; use crate::types::transaction::TxType; use crate::types::validity_predicate::EvalVp; + use crate::vm::host_env::TxRuntimeError; use crate::vm::wasm; const TX_GAS_LIMIT: u64 = 10_000_000_000; + /// Test that we sanitize accesses to invalid addresses in wasm memory. + #[test] + fn test_tx_sanitize_invalid_addrs() { + let tx_code = wasmer::wat2wasm( + r#" + (module + (import "env" "namada_tx_read" (func (param i64 i64) (result i64))) + (func (param i64 i64) + i64.const 18446744073709551615 + i64.const 1 + (call 0) + drop + ) + (memory 16) + (export "memory" (memory 0)) + (export "_apply_tx" (func 1)) + ) + "# + .as_bytes(), + ) + .expect("unexpected error converting wat2wasm") + .into_owned(); + + const PANIC_MSG: &str = + "Test should have failed with a wasm runtime memory error"; + + let error = execute_tx_with_code(tx_code).expect_err(PANIC_MSG); + assert!( + matches!( + assert_rt_mem_error(&error, PANIC_MSG), + memory::Error::OverflowingOffset(18446744073709551615, 1), + ), + "{PANIC_MSG}" + ); + } + + /// Extract a wasm runtime memory error from some [`Error`]. + fn assert_rt_mem_error<'err>( + error: &'err Error, + assert_msg: &str, + ) -> &'err memory::Error { + let Error::RuntimeError(rt_error) = error else { + panic!("{assert_msg}: {error}"); + }; + let source_err = + rt_error.source().expect("No runtime error source found"); + let downcasted_tx_rt_err: &TxRuntimeError = source_err + .downcast_ref() + .unwrap_or_else(|| panic!("{assert_msg}: {source_err}")); + let TxRuntimeError::MemoryError(tx_mem_err) = downcasted_tx_rt_err else { + panic!("{assert_msg}: {downcasted_tx_rt_err}"); + }; + tx_mem_err + .downcast_ref() + .unwrap_or_else(|| panic!("{assert_msg}: {tx_mem_err}")) + } + /// Test that when a transaction wasm goes over the stack-height limit, the /// execution is aborted. #[test] @@ -1247,6 +1307,40 @@ mod tests { assert!(!passed); } + fn execute_tx_with_code(tx_code: Vec) -> Result> { + let tx_data = vec![]; + let tx_index = TxIndex::default(); + let storage = TestStorage::default(); + let mut write_log = WriteLog::default(); + let mut gas_meter = TxGasMeter::new_from_sub_limit(TX_GAS_LIMIT.into()); + let (mut vp_cache, _) = + wasm::compilation_cache::common::testing::cache(); + let (mut tx_cache, _) = + wasm::compilation_cache::common::testing::cache(); + + // store the tx code + let code_hash = Hash::sha256(&tx_code); + let code_len = (tx_code.len() as u64).serialize_to_vec(); + let key = Key::wasm_code(&code_hash); + let len_key = Key::wasm_code_len(&code_hash); + write_log.write(&key, tx_code).unwrap(); + write_log.write(&len_key, code_len).unwrap(); + + let mut outer_tx = Tx::from_type(TxType::Raw); + outer_tx.set_code(Code::from_hash(code_hash, None)); + outer_tx.set_data(Data::new(tx_data)); + + tx( + &storage, + &mut write_log, + &mut gas_meter, + &tx_index, + &outer_tx, + &mut vp_cache, + &mut tx_cache, + ) + } + fn loop_in_tx_wasm(loops: u32) -> Result> { // A transaction with a recursive loop. // The boilerplate code is generated from tx_template.wasm using @@ -1282,37 +1376,7 @@ mod tests { .expect("unexpected error converting wat2wasm") .into_owned(); - let tx_data = vec![]; - let tx_index = TxIndex::default(); - let storage = TestStorage::default(); - let mut write_log = WriteLog::default(); - let mut gas_meter = TxGasMeter::new_from_sub_limit(TX_GAS_LIMIT.into()); - let (mut vp_cache, _) = - wasm::compilation_cache::common::testing::cache(); - let (mut tx_cache, _) = - wasm::compilation_cache::common::testing::cache(); - - // store the tx code - let code_hash = Hash::sha256(&tx_code); - let code_len = (tx_code.len() as u64).serialize_to_vec(); - let key = Key::wasm_code(&code_hash); - let len_key = Key::wasm_code_len(&code_hash); - write_log.write(&key, tx_code).unwrap(); - write_log.write(&len_key, code_len).unwrap(); - - let mut outer_tx = Tx::from_type(TxType::Raw); - outer_tx.set_code(Code::from_hash(code_hash, None)); - outer_tx.set_data(Data::new(tx_data)); - - tx( - &storage, - &mut write_log, - &mut gas_meter, - &tx_index, - &outer_tx, - &mut vp_cache, - &mut tx_cache, - ) + execute_tx_with_code(tx_code) } fn loop_in_vp_wasm(loops: u32) -> Result { From bfff15109eee6a51d8ed00a9baa90a1b6e1a3604 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Wed, 20 Dec 2023 12:52:16 +0000 Subject: [PATCH 3/3] Changelog for #2385 --- .changelog/unreleased/bug-fixes/2385-wasm-vm-addr-sanitize.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/2385-wasm-vm-addr-sanitize.md diff --git a/.changelog/unreleased/bug-fixes/2385-wasm-vm-addr-sanitize.md b/.changelog/unreleased/bug-fixes/2385-wasm-vm-addr-sanitize.md new file mode 100644 index 00000000000..4f80c7153b2 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/2385-wasm-vm-addr-sanitize.md @@ -0,0 +1,3 @@ +- Sanitize wasm memory accesses which are outside of the 32-bit address + range, to avoid crashing the ledger while executing malicious wasm payloads. + ([\#2385](https://github.com/anoma/namada/pull/2385)) \ No newline at end of file