Merge branch 'tiago/wasm-vm-addr-sanitize' (#2385)

* origin/tiago/wasm-vm-addr-sanitize:
  Changelog for #2385
  Add wasm ptr overflow checks regression test
  Sanitize tx and vp wasm memory accesses

.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))

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(())
 }

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<u8>) -> Result<BTreeSet<Address>> {
+        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<BTreeSet<Address>> {
         // 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<bool> {