diff --git a/frame/evm/precompile/shielded-pool/CHANGELOG.md b/frame/evm/precompile/shielded-pool/CHANGELOG.md index ce0e1695..055c8f4e 100644 --- a/frame/evm/precompile/shielded-pool/CHANGELOG.md +++ b/frame/evm/precompile/shielded-pool/CHANGELOG.md @@ -2,6 +2,68 @@ All notable changes to `pallet-evm-precompile-shielded-pool` will be documented in this file. +## [0.5.0] - 2026-08-07 + +### Security + +- **ABI decoder rejects oversized words instead of truncating them.** Offsets and + lengths are `uint256`, but `read_offset` and `read_length` narrowed them with + `low_u32()`, which keeps the bottom 32 bits and discards the rest. An offset of + `2^32 + 8` read back as `8`, and `2^32` as `0` — so the bounds check validated a + value the sender never wrote and the decoder indexed somewhere else entirely. + Every word is now checked against `usize::MAX` and refused if it does not fit. + + `decode_u32` had the same shape: the ABI declares the argument as `uint32`, so a + wider word is a call the callee never agreed to. Keeping the low bits turned a + malformed call into a plausible one naming a different value. + +- **Offset and length arithmetic is checked.** `data_start + length > + params.len()` wraps for a large length, and the wrapped sum passes the very + bounds check it was meant to fail. Release builds do not enable + `overflow-checks`, so it wrapped silently on exactly the input being guarded + against. Same for `count * 32` in the array decoders. All now use + `checked_add` / `checked_mul`. The wrapping-length case was worse than a + mis-decode: it built an inverted slice range and panicked the runtime + (`slice index starts at 128 but ends at 127` → `wasm unreachable` trap), + reachable from an unsigned, gas-free `eth_call`. + +- **Element counts are validated before allocating.** `Vec::with_capacity(count)` + reserved from a calldata-supplied count before anything confirmed the buffer + could hold that many elements. The span is now bounds-checked first. + +- A `.unwrap()` on a slice conversion in `decode_bytes32_array_at_slot` became a + propagated error. It was unreachable given the surrounding checks, but a panic + in a precompile is not a failure mode worth keeping reachable-by-accident. + +### Changed + +- `abi.rs` and `dispatch.rs` split into directories by responsibility, no + behaviour change. `abi/` → `guard` (checked arithmetic), `scalar` (`uint32`, + `bytes32`), `dynamic` (`bytes`, `bytes32[]`, `bytes[]`). `dispatch/` → `mod` + (shared call/gas/result handling in `record_and_dispatch`) and `origin` (the + three origin modes). Tests moved alongside the code they cover. + +### Notes + +- No ABI change. Every selector, parameter and head layout is untouched, and + well-formed calldata decodes exactly as before. What narrowed is the set of + malformed inputs the decoder will act on. + +### Verification +74 precompile tests (9 new), clippy clean across the workspace under the CI +feature set. + +Each guard was verified by reverting it and confirming the tests fail: replacing +`word_to_usize` with `low_u32` breaks three, and turning one `checked_add` into +`wrapping_add` breaks another — that last one is the direct demonstration, since +without the check the crafted length passes the bounds check. + +A dev-node run (26/26) sends hand-built calldata straight at the precompile via +`eth_call`: offsets of `2^32`, `2^64`, `2^255` and `uint256::MAX`, lengths that +would wrap the bounds check, over-wide `uint32` words, truncated heads, and a +40-call hostile burst — checking block height after each batch. Well-formed +calldata still clears the decoder and reaches the pallet's own guards. + ## [0.4.0] - 2026-08-06 ### Changed diff --git a/frame/evm/precompile/shielded-pool/Cargo.toml b/frame/evm/precompile/shielded-pool/Cargo.toml index f8dea42e..c589b199 100644 --- a/frame/evm/precompile/shielded-pool/Cargo.toml +++ b/frame/evm/precompile/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-evm-precompile-shielded-pool" -version = "0.4.0" +version = "0.5.0" authors = { workspace = true } edition = "2021" description = "EVM Precompile for Orbinum Shielded Pool Pallet." diff --git a/frame/evm/precompile/shielded-pool/src/abi.rs b/frame/evm/precompile/shielded-pool/src/abi.rs deleted file mode 100644 index 591cf078..00000000 --- a/frame/evm/precompile/shielded-pool/src/abi.rs +++ /dev/null @@ -1,129 +0,0 @@ -//! Pure ABI decoding helpers for the shielded-pool precompile. -//! -//! All functions are stateless free functions — no FRAME generics, no pallet types. -//! They operate directly on raw ABI-encoded byte slices and return typed Rust values. - -use alloc::vec::Vec; - -use fp_evm::{ExitError, PrecompileFailure}; -use sp_core::U256; - -// ───────────────────────────────────────────────────────────────────────────── -// Scalar decoders -// ───────────────────────────────────────────────────────────────────────────── - -/// Decodes a `uint32` from a 32-byte ABI slot (big-endian, right-aligned). -pub fn decode_u32(slot: &[u8]) -> Result { - if slot.len() < 32 { - return Err(abi_error("uint32 slot too short")); - } - Ok(U256::from_big_endian(&slot[0..32]).low_u32()) -} - -/// Reads the 32-byte value at `params[slot_start..slot_start+32]` verbatim. -pub fn read_bytes32(params: &[u8], slot_start: usize) -> Result<[u8; 32], PrecompileFailure> { - if slot_start + 32 > params.len() { - return Err(abi_error("bytes32 slot out of bounds")); - } - params[slot_start..slot_start + 32] - .try_into() - .map_err(|_| abi_error("bytes32 copy failed")) -} - -// ───────────────────────────────────────────────────────────────────────────── -// Dynamic-type decoders -// ───────────────────────────────────────────────────────────────────────────── - -/// Decodes a dynamic `bytes` value. -/// -/// `slot_start` is the byte offset inside `params` where the 32-byte offset -/// pointer for the bytes value lives (standard ABI head encoding). -pub fn decode_bytes_at_slot( - params: &[u8], - slot_start: usize, -) -> Result, PrecompileFailure> { - let offset = read_offset(params, slot_start)?; - let length = read_length(params, offset)?; - let data_start = offset + 32; - if data_start + length > params.len() { - return Err(abi_error("bytes data out of bounds")); - } - Ok(params[data_start..data_start + length].to_vec()) -} - -/// Decodes a `bytes32[]` whose offset pointer lives at `slot_start` in `params`. -pub fn decode_bytes32_array_at_slot( - params: &[u8], - slot_start: usize, -) -> Result, PrecompileFailure> { - let offset = read_offset(params, slot_start)?; - let count = read_length(params, offset)?; - let data_start = offset + 32; - if data_start + count * 32 > params.len() { - return Err(abi_error("bytes32[] data out of bounds")); - } - let mut items = Vec::with_capacity(count); - for i in 0..count { - let s = data_start + i * 32; - let elem: [u8; 32] = params[s..s + 32].try_into().unwrap(); - items.push(elem); - } - Ok(items) -} - -/// Decodes a `bytes[]` whose offset pointer lives at `slot_start` in `params`. -pub fn decode_bytes_array_at_slot( - params: &[u8], - slot_start: usize, -) -> Result>, PrecompileFailure> { - let array_offset = read_offset(params, slot_start)?; - let count = read_length(params, array_offset)?; - // Each element has a relative offset pointer from the start of the array-data region - // (= array_offset + 32, which is right after the length word). - let data_base = array_offset + 32; - - let mut result = Vec::with_capacity(count); - for i in 0..count { - let rel_offset_pos = data_base + i * 32; - if rel_offset_pos + 32 > params.len() { - return Err(abi_error("bytes[] element offset out of bounds")); - } - let rel_offset = - U256::from_big_endian(¶ms[rel_offset_pos..rel_offset_pos + 32]).low_u32() as usize; - let abs_offset = data_base + rel_offset; - let elem_len = read_length(params, abs_offset)?; - let elem_start = abs_offset + 32; - if elem_start + elem_len > params.len() { - return Err(abi_error("bytes[] element data out of bounds")); - } - result.push(params[elem_start..elem_start + elem_len].to_vec()); - } - Ok(result) -} - -// ───────────────────────────────────────────────────────────────────────────── -// Internal helpers -// ───────────────────────────────────────────────────────────────────────────── - -/// Reads an ABI offset (usize) from a 32-byte slot at `slot_start` in `params`. -fn read_offset(params: &[u8], slot_start: usize) -> Result { - if slot_start + 32 > params.len() { - return Err(abi_error("slot out of bounds")); - } - Ok(U256::from_big_endian(¶ms[slot_start..slot_start + 32]).low_u32() as usize) -} - -/// Reads an ABI length word (usize) from `params[offset..offset+32]`. -fn read_length(params: &[u8], offset: usize) -> Result { - if offset + 32 > params.len() { - return Err(abi_error("length word out of bounds")); - } - Ok(U256::from_big_endian(¶ms[offset..offset + 32]).low_u32() as usize) -} - -/// Constructs a `PrecompileFailure::Error` with the given message. -fn abi_error(msg: &'static str) -> PrecompileFailure { - PrecompileFailure::Error { - exit_status: ExitError::Other(msg.into()), - } -} diff --git a/frame/evm/precompile/shielded-pool/src/abi/dynamic.rs b/frame/evm/precompile/shielded-pool/src/abi/dynamic.rs new file mode 100644 index 00000000..4c660dab --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/abi/dynamic.rs @@ -0,0 +1,220 @@ +//! Dynamic ABI decoders: `bytes`, `bytes32[]`, `bytes[]`. +//! +//! Each reads an offset pointer from the head, then a length/count word at that +//! offset, then the data region — every step bounds-checked through [`super::guard`]. + +use alloc::vec::Vec; + +use fp_evm::PrecompileFailure; +use sp_core::U256; + +use super::guard::{abi_error, checked_add, checked_mul, checked_range, word_to_usize}; + +/// Decodes a dynamic `bytes` value. +/// +/// `slot_start` is the byte offset inside `params` where the 32-byte offset +/// pointer for the bytes value lives (standard ABI head encoding). +pub fn decode_bytes_at_slot( + params: &[u8], + slot_start: usize, +) -> Result, PrecompileFailure> { + let offset = read_offset(params, slot_start)?; + let length = read_length(params, offset)?; + // The length word sits at `offset`, so the data starts 32 bytes later. + let data_start = checked_add(offset, 32, "bytes offset overflows")?; + let range = checked_range(data_start, length, params.len(), "bytes data")?; + Ok(params[range].to_vec()) +} + +/// Decodes a `bytes32[]` whose offset pointer lives at `slot_start` in `params`. +pub fn decode_bytes32_array_at_slot( + params: &[u8], + slot_start: usize, +) -> Result, PrecompileFailure> { + let offset = read_offset(params, slot_start)?; + let count = read_length(params, offset)?; + let data_start = checked_add(offset, 32, "bytes32[] offset overflows")?; + // `count * 32` is the whole span. Bounds-checking it here rejects an oversized + // array before `with_capacity` reserves for a count that comes from calldata. + let span = checked_mul(count, 32, "bytes32[] count overflows")?; + let range = checked_range(data_start, span, params.len(), "bytes32[]")?; + let data_start = range.start; + let mut items = Vec::with_capacity(count); + for i in 0..count { + let s = data_start + i * 32; // bounded by the checked span above + let elem: [u8; 32] = params[s..s + 32] + .try_into() + .map_err(|_| abi_error("bytes32[] element copy failed"))?; + items.push(elem); + } + Ok(items) +} + +/// Decodes a `bytes[]` whose offset pointer lives at `slot_start` in `params`. +pub fn decode_bytes_array_at_slot( + params: &[u8], + slot_start: usize, +) -> Result>, PrecompileFailure> { + let array_offset = read_offset(params, slot_start)?; + let count = read_length(params, array_offset)?; + // Each element has a relative offset pointer from the start of the array-data + // region (= array_offset + 32, right after the length word). + let data_base = checked_add(array_offset, 32, "bytes[] offset overflows")?; + + // Each element needs a 32-byte pointer. Bounds-checking the pointer table here + // keeps a declared count of 2^30 from reserving gigabytes in `with_capacity` + // on input that could never decode. + let pointers = checked_mul(count, 32, "bytes[] count overflows")?; + checked_range(data_base, pointers, params.len(), "bytes[] pointer table")?; + + let mut result = Vec::with_capacity(count); + for i in 0..count { + let rel_offset_pos = data_base + i * 32; // bounded by the pointer table above + let rel_offset = word_to_usize( + U256::from_big_endian(¶ms[rel_offset_pos..rel_offset_pos + 32]), + "bytes[] element offset does not fit in usize", + )?; + let abs_offset = checked_add(data_base, rel_offset, "bytes[] element offset overflows")?; + let elem_len = read_length(params, abs_offset)?; + let elem_start = checked_add(abs_offset, 32, "bytes[] element start overflows")?; + let range = checked_range(elem_start, elem_len, params.len(), "bytes[] element data")?; + result.push(params[range].to_vec()); + } + Ok(result) +} + +/// Reads an ABI offset from a 32-byte slot at `slot_start` in `params`. +fn read_offset(params: &[u8], slot_start: usize) -> Result { + let range = checked_range(slot_start, 32, params.len(), "offset slot")?; + word_to_usize( + U256::from_big_endian(¶ms[range]), + "offset does not fit in usize", + ) +} + +/// Reads an ABI length word from `params[offset..offset+32]`. +fn read_length(params: &[u8], offset: usize) -> Result { + let range = checked_range(offset, 32, params.len(), "length word")?; + word_to_usize( + U256::from_big_endian(¶ms[range]), + "length does not fit in usize", + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A 32-byte big-endian ABI word holding `v`. + fn word(v: U256) -> [u8; 32] { + v.to_big_endian() + } + + /// Head slot pointing at `offset`, followed by padding out to `total` bytes. + fn with_offset(offset: U256, total: usize) -> Vec { + let mut p = word(offset).to_vec(); + p.resize(total.max(32), 0); + p + } + + // ─── Truncation ────────────────────────────────────────────────────────── + + /// The attack `low_u32` allowed: a word whose low 32 bits look benign while + /// the value itself is astronomically out of range. `2^32 + 8` read back as + /// `8`, so the bounds check validated an offset nobody sent. + #[test] + fn an_offset_above_u32_is_refused_not_truncated() { + let sneaky = U256::from(1u64 << 32) + U256::from(8u64); + assert_eq!( + sneaky.low_u32(), + 8, + "the low bits must look valid, or this proves nothing" + ); + + let params = with_offset(sneaky, 128); + assert!(decode_bytes_at_slot(¶ms, 0).is_err()); + } + + /// `2^32` truncates to zero, which points the reader at the head itself. + #[test] + fn an_offset_of_exactly_two_to_the_32_is_refused() { + let sneaky = U256::from(1u64 << 32); + assert_eq!(sneaky.low_u32(), 0); + + let params = with_offset(sneaky, 128); + assert!(decode_bytes_at_slot(¶ms, 0).is_err()); + } + + // ─── Overflow ──────────────────────────────────────────────────────────── + + /// `data_start + length` on plain arithmetic wraps for a huge length, and the + /// wrapped sum passes the bounds check it was meant to fail. Release builds + /// do not enable overflow-checks, so it would wrap silently. + #[test] + fn a_length_that_would_wrap_the_bounds_check_is_refused() { + let mut params = word(U256::from(32u64)).to_vec(); // offset -> 32 + params.extend_from_slice(&word(U256::from(usize::MAX))); // length + params.resize(128, 0); + + assert!(decode_bytes_at_slot(¶ms, 0).is_err()); + } + + /// `count * 32` wraps the same way. 2^59 elements multiplies past usize on a + /// 64-bit host and lands back inside the buffer. + #[test] + fn an_element_count_that_would_wrap_is_refused() { + let mut params = word(U256::from(32u64)).to_vec(); + params.extend_from_slice(&word(U256::from(1u64 << 59))); + params.resize(256, 0); + + assert!(decode_bytes32_array_at_slot(¶ms, 0).is_err()); + } + + /// An element count far past what the buffer can hold must be refused + /// *before* `Vec::with_capacity` reserves for it. + #[test] + fn an_element_count_larger_than_the_buffer_is_refused_early() { + let mut params = word(U256::from(32u64)).to_vec(); + params.extend_from_slice(&word(U256::from(1u64 << 30))); // ~1e9 elements + params.resize(256, 0); + + assert!(decode_bytes_array_at_slot(¶ms, 0).is_err()); + assert!(decode_bytes32_array_at_slot(¶ms, 0).is_err()); + } + + // ─── Well-formed input still decodes ───────────────────────────────────── + + /// The guards must not reject real calldata, or they trade one bug for a + /// worse one. + #[test] + fn a_well_formed_bytes_value_still_decodes() { + let payload = [0xABu8; 5]; + let mut params = word(U256::from(32u64)).to_vec(); // offset + params.extend_from_slice(&word(U256::from(payload.len()))); // length + params.extend_from_slice(&payload); + params.resize(96, 0); // pad to a 32-byte boundary + + assert_eq!(decode_bytes_at_slot(¶ms, 0).unwrap(), payload.to_vec()); + } + + #[test] + fn a_well_formed_bytes32_array_still_decodes() { + let mut params = word(U256::from(32u64)).to_vec(); // offset + params.extend_from_slice(&word(U256::from(2u64))); // count + params.extend_from_slice(&[0x11u8; 32]); + params.extend_from_slice(&[0x22u8; 32]); + + let out = decode_bytes32_array_at_slot(¶ms, 0).unwrap(); + assert_eq!(out, alloc::vec![[0x11u8; 32], [0x22u8; 32]]); + } + + /// An empty dynamic array is valid ABI and must survive the count checks. + #[test] + fn an_empty_array_still_decodes() { + let mut params = word(U256::from(32u64)).to_vec(); + params.extend_from_slice(&word(U256::zero())); + + assert!(decode_bytes32_array_at_slot(¶ms, 0).unwrap().is_empty()); + assert!(decode_bytes_array_at_slot(¶ms, 0).unwrap().is_empty()); + } +} diff --git a/frame/evm/precompile/shielded-pool/src/abi/guard.rs b/frame/evm/precompile/shielded-pool/src/abi/guard.rs new file mode 100644 index 00000000..8eba7767 --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/abi/guard.rs @@ -0,0 +1,66 @@ +//! Bounds- and overflow-checked arithmetic over attacker-chosen calldata. +//! +//! Offsets, lengths and counts all come from calldata, so every `+`/`*` on them +//! is checked: release builds do not enable `overflow-checks`, and a wrapped sum +//! passes the very bounds check meant to reject it. This module is the only place +//! that arithmetic lives, and the only place that reads a 256-bit word as a `usize`. + +use fp_evm::{ExitError, PrecompileFailure}; +use sp_core::U256; + +/// Constructs a `PrecompileFailure::Error` with the given message. +pub(super) fn abi_error(msg: &'static str) -> PrecompileFailure { + PrecompileFailure::Error { + exit_status: ExitError::Other(msg.into()), + } +} + +/// Checked `a + b`, mapping overflow to an ABI error labelled `what`. +pub(super) fn checked_add( + a: usize, + b: usize, + what: &'static str, +) -> Result { + a.checked_add(b).ok_or_else(|| abi_error(what)) +} + +/// Checked `a * b`, mapping overflow to an ABI error labelled `what`. +pub(super) fn checked_mul( + a: usize, + b: usize, + what: &'static str, +) -> Result { + a.checked_mul(b).ok_or_else(|| abi_error(what)) +} + +/// The range `start..start + span`, verified to fit inside a buffer of `params_len` +/// without wrapping. `what` labels the error on both the overflow and the +/// out-of-bounds path. +pub(super) fn checked_range( + start: usize, + span: usize, + params_len: usize, + what: &'static str, +) -> Result, PrecompileFailure> { + let end = start.checked_add(span).ok_or_else(|| abi_error(what))?; + if end > params_len { + return Err(abi_error(what)); + } + Ok(start..end) +} + +/// Reads a 256-bit ABI word as a `usize`, rejecting anything that does not fit. +/// +/// ABI offsets and lengths are `uint256`. Narrowing one with `low_u32` keeps the +/// bottom 32 bits and silently discards the rest, so `2^32 + 8` reads back as +/// `8` and `2^32` as `0` — the caller then bounds-checks a value the sender +/// never wrote, and indexes somewhere else entirely. +/// +/// `usize` is 32-bit under Wasm and 64-bit natively, so `try_into` also keeps +/// the two from disagreeing about which calldata is acceptable. +pub(super) fn word_to_usize(word: U256, what: &'static str) -> Result { + if word > U256::from(usize::MAX) { + return Err(abi_error(what)); + } + usize::try_from(word).map_err(|_| abi_error(what)) +} diff --git a/frame/evm/precompile/shielded-pool/src/abi/mod.rs b/frame/evm/precompile/shielded-pool/src/abi/mod.rs new file mode 100644 index 00000000..e56af8c5 --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/abi/mod.rs @@ -0,0 +1,23 @@ +//! Pure ABI decoding helpers for the shielded-pool precompile. +//! +//! All functions are stateless free functions — no FRAME generics, no pallet types. +//! They operate directly on raw ABI-encoded byte slices and return typed Rust values. +//! +//! Every input here is attacker-chosen calldata, so two rules hold throughout: +//! a 256-bit ABI word is **rejected** when it does not fit the type it is being +//! read into rather than narrowed to its low bits, and every offset and length +//! arithmetic is checked. Release builds do not enable `overflow-checks`, so an +//! unchecked sum would wrap silently and pass the very bounds check meant to +//! catch it. +//! +//! Split by responsibility: +//! - [`guard`] — the checked arithmetic and `usize` conversion every decoder relies on. +//! - [`scalar`] — fixed-width types (`uint32`, `bytes32`). +//! - [`dynamic`] — offset/length-driven types (`bytes`, `bytes32[]`, `bytes[]`). + +mod dynamic; +mod guard; +mod scalar; + +pub use dynamic::{decode_bytes32_array_at_slot, decode_bytes_array_at_slot, decode_bytes_at_slot}; +pub use scalar::{decode_u32, read_bytes32}; diff --git a/frame/evm/precompile/shielded-pool/src/abi/scalar.rs b/frame/evm/precompile/shielded-pool/src/abi/scalar.rs new file mode 100644 index 00000000..9160235d --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/abi/scalar.rs @@ -0,0 +1,54 @@ +//! Fixed-width ABI decoders: `uint32` and `bytes32`. + +use fp_evm::PrecompileFailure; +use sp_core::U256; + +use super::guard::{abi_error, checked_range}; + +/// Decodes a `uint32` from a 32-byte ABI slot (big-endian, right-aligned). +/// +/// A word wider than `u32` is rejected rather than truncated: the ABI declares +/// the argument as `uint32`, so a caller sending more has encoded something the +/// callee never agreed to, and silently keeping the low bits turns a malformed +/// call into a plausible one with a different value. +pub fn decode_u32(slot: &[u8]) -> Result { + if slot.len() < 32 { + return Err(abi_error("uint32 slot too short")); + } + let word = U256::from_big_endian(&slot[0..32]); + if word > U256::from(u32::MAX) { + return Err(abi_error("uint32 slot exceeds u32")); + } + Ok(word.low_u32()) +} + +/// Reads the 32-byte value at `params[slot_start..slot_start+32]` verbatim. +pub fn read_bytes32(params: &[u8], slot_start: usize) -> Result<[u8; 32], PrecompileFailure> { + let range = checked_range(slot_start, 32, params.len(), "bytes32 slot")?; + params[range] + .try_into() + .map_err(|_| abi_error("bytes32 copy failed")) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A 32-byte big-endian ABI word holding `v`. + fn word(v: U256) -> [u8; 32] { + v.to_big_endian() + } + + /// The ABI declares `uint32`, so a wider word is a call the callee never + /// agreed to — keeping the low bits would turn it into a different, valid + /// looking argument. + #[test] + fn a_uint32_slot_wider_than_u32_is_refused() { + let sneaky = U256::from(1u64 << 32) + U256::from(7u64); + assert_eq!(sneaky.low_u32(), 7); + assert!(decode_u32(&word(sneaky)).is_err()); + + // u32::MAX itself still decodes. + assert_eq!(decode_u32(&word(U256::from(u32::MAX))).unwrap(), u32::MAX); + } +} diff --git a/frame/evm/precompile/shielded-pool/src/dispatch.rs b/frame/evm/precompile/shielded-pool/src/dispatch.rs deleted file mode 100644 index 326c7785..00000000 --- a/frame/evm/precompile/shielded-pool/src/dispatch.rs +++ /dev/null @@ -1,138 +0,0 @@ -//! Dispatch helpers for the shielded-pool precompile. -//! -//! Two dispatch modes mirror the two pallet extrinsic origin checks: -//! -//! - [`from_self`] — dispatches with the **precompile's own address** as signed origin. -//! Used for `shield` (payable): the EVM executor already transferred `msg.value` from -//! the caller to the precompile address, so the pallet moves those funds from the -//! precompile account to the pool without touching the caller a second time. -//! -//! - [`unsigned`] — dispatches with `None` origin (`ensure_none`). -//! Used for `private_transfer` and `unshield`, where a ZK proof authenticates the -//! operation and no transaction signer is needed. - -use alloc::format; - -use fp_evm::{ - ExitError, ExitSucceed, PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileResult, -}; -use frame_support::dispatch::GetDispatchInfo; -use pallet_evm::{AddressMapping, GasWeightMapping}; -use sp_runtime::traits::Dispatchable; - -/// Dispatches `call` with the **precompile's own address** as signed origin. -/// -/// Gas cost is derived from the call's dispatch weight via [`GasWeightMapping`]. -pub fn from_self( - handle: &mut impl PrecompileHandle, - call: pallet_shielded_pool::Call, -) -> PrecompileResult -where - T: pallet_evm::Config + pallet_shielded_pool::Config, - ::RuntimeCall: Dispatchable - + GetDispatchInfo - + From>, - <::RuntimeCall as Dispatchable>::RuntimeOrigin: - From::AccountId>>, - <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, - pallet_evm::AccountIdOf: Into<::AccountId>, -{ - let runtime_call = <::RuntimeCall as From< - pallet_shielded_pool::Call, - >>::from(call); - let gas_cost = - T::GasWeightMapping::weight_to_gas(runtime_call.get_dispatch_info().total_weight()); - handle.record_cost(gas_cost)?; - - let self_account: ::AccountId = - T::AddressMapping::into_account_id(handle.context().address).into(); - let origin = <::RuntimeCall as Dispatchable>::RuntimeOrigin::from( - Some(self_account), - ); - - dispatch(runtime_call, origin) -} - -/// Dispatches `call` with the **EVM caller** as signed origin. -/// -/// Used for `claim_shielded_fees`: the validator calls the precompile from their -/// EVM address; their `H160` is mapped to an `AccountId` via `AddressMapping` and -/// used as the signed origin so `ensure_signed` succeeds in the pallet. -pub fn from_caller( - handle: &mut impl PrecompileHandle, - call: pallet_shielded_pool::Call, -) -> PrecompileResult -where - T: pallet_evm::Config + pallet_shielded_pool::Config, - ::RuntimeCall: Dispatchable - + GetDispatchInfo - + From>, - <::RuntimeCall as Dispatchable>::RuntimeOrigin: - From::AccountId>>, - <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, - pallet_evm::AccountIdOf: Into<::AccountId>, -{ - let runtime_call = <::RuntimeCall as From< - pallet_shielded_pool::Call, - >>::from(call); - let gas_cost = - T::GasWeightMapping::weight_to_gas(runtime_call.get_dispatch_info().total_weight()); - handle.record_cost(gas_cost)?; - - let caller_account: ::AccountId = - T::AddressMapping::into_account_id(handle.context().caller).into(); - let origin = <::RuntimeCall as Dispatchable>::RuntimeOrigin::from( - Some(caller_account), - ); - - dispatch(runtime_call, origin) -} - -/// Dispatches `call` with `None` origin (`ensure_none`). -/// -/// Gas cost is derived from the call's dispatch weight via [`GasWeightMapping`]. -pub fn unsigned( - handle: &mut impl PrecompileHandle, - call: pallet_shielded_pool::Call, -) -> PrecompileResult -where - T: pallet_evm::Config + pallet_shielded_pool::Config, - ::RuntimeCall: Dispatchable - + GetDispatchInfo - + From>, - <::RuntimeCall as Dispatchable>::RuntimeOrigin: - From::AccountId>>, - <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, -{ - let runtime_call = <::RuntimeCall as From< - pallet_shielded_pool::Call, - >>::from(call); - let gas_cost = - T::GasWeightMapping::weight_to_gas(runtime_call.get_dispatch_info().total_weight()); - handle.record_cost(gas_cost)?; - - let origin = - <::RuntimeCall as Dispatchable>::RuntimeOrigin::from(None); - - dispatch(runtime_call, origin) -} - -// ───────────────────────────────────────────────────────────────────────────── -// Internal -// ───────────────────────────────────────────────────────────────────────────── - -fn dispatch(call: C, origin: C::RuntimeOrigin) -> PrecompileResult -where - C: Dispatchable, - C::PostInfo: core::fmt::Debug, -{ - match call.dispatch(origin) { - Ok(_) => Ok(PrecompileOutput { - exit_status: ExitSucceed::Stopped, - output: Default::default(), - }), - Err(e) => Err(PrecompileFailure::Error { - exit_status: ExitError::Other(format!("{e:?}").into()), - }), - } -} diff --git a/frame/evm/precompile/shielded-pool/src/dispatch/mod.rs b/frame/evm/precompile/shielded-pool/src/dispatch/mod.rs new file mode 100644 index 00000000..dee70aac --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/dispatch/mod.rs @@ -0,0 +1,68 @@ +//! Dispatch helpers for the shielded-pool precompile. +//! +//! Three dispatch modes mirror the pallet's extrinsic origin checks. They differ +//! only in the origin they build; everything else — converting the pallet call to +//! a runtime call, charging gas from its weight, mapping the dispatch result to a +//! precompile outcome — is shared here and reached through [`record_and_dispatch`]. +//! +//! - [`origin::from_self`] — the precompile's own address as signed origin. +//! - [`origin::from_caller`] — the EVM caller's mapped address as signed origin. +//! - [`origin::unsigned`] — `None` origin (`ensure_none`). + +mod origin; + +pub use origin::{from_caller, from_self, unsigned}; + +use alloc::format; + +use fp_evm::{ + ExitError, ExitSucceed, PrecompileFailure, PrecompileHandle, PrecompileOutput, PrecompileResult, +}; +use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo}; +use pallet_evm::GasWeightMapping; +use sp_runtime::traits::Dispatchable; + +/// The runtime origin type for `T`'s runtime call. +type RuntimeOriginOf = <::RuntimeCall as Dispatchable>::RuntimeOrigin; + +/// Converts the pallet call to a runtime call, charges its gas, dispatches it with +/// the origin from `build_origin`, and maps the result to a precompile outcome. Each +/// mode in [`origin`] just builds its origin and hands it here — the rest is shared. +fn record_and_dispatch( + handle: &mut impl PrecompileHandle, + call: pallet_shielded_pool::Call, + build_origin: impl FnOnce() -> RuntimeOriginOf, +) -> PrecompileResult +where + T: pallet_evm::Config + pallet_shielded_pool::Config, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, +{ + let runtime_call = <::RuntimeCall as From< + pallet_shielded_pool::Call, + >>::from(call); + let gas_cost = + T::GasWeightMapping::weight_to_gas(runtime_call.get_dispatch_info().total_weight()); + handle.record_cost(gas_cost)?; + + dispatch(runtime_call, build_origin()) +} + +/// Dispatches an already-built runtime call and maps its result. +fn dispatch(call: C, origin: C::RuntimeOrigin) -> PrecompileResult +where + C: Dispatchable, + C::PostInfo: core::fmt::Debug, +{ + match call.dispatch(origin) { + Ok(_) => Ok(PrecompileOutput { + exit_status: ExitSucceed::Stopped, + output: Default::default(), + }), + Err(e) => Err(PrecompileFailure::Error { + exit_status: ExitError::Other(format!("{e:?}").into()), + }), + } +} diff --git a/frame/evm/precompile/shielded-pool/src/dispatch/origin.rs b/frame/evm/precompile/shielded-pool/src/dispatch/origin.rs new file mode 100644 index 00000000..12a59bb4 --- /dev/null +++ b/frame/evm/precompile/shielded-pool/src/dispatch/origin.rs @@ -0,0 +1,80 @@ +//! The three dispatch modes, one per pallet origin check. Each builds its origin +//! and defers the shared work to [`super::record_and_dispatch`]. + +use fp_evm::{PrecompileHandle, PrecompileResult}; +use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo}; +use pallet_evm::AddressMapping; +use sp_runtime::traits::Dispatchable; + +use super::{record_and_dispatch, RuntimeOriginOf}; + +/// Dispatches `call` with the **precompile's own address** as signed origin. +/// +/// Used for `shield` (payable): the EVM executor already transferred `msg.value` +/// from the caller to the precompile address, so the pallet moves those funds from +/// the precompile account to the pool without touching the caller a second time. +pub fn from_self( + handle: &mut impl PrecompileHandle, + call: pallet_shielded_pool::Call, +) -> PrecompileResult +where + T: pallet_evm::Config + pallet_shielded_pool::Config, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + RuntimeOriginOf: From::AccountId>>, + <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, + pallet_evm::AccountIdOf: Into<::AccountId>, +{ + let address = handle.context().address; + record_and_dispatch(handle, call, || { + let account: ::AccountId = + T::AddressMapping::into_account_id(address).into(); + RuntimeOriginOf::::from(Some(account)) + }) +} + +/// Dispatches `call` with the **EVM caller** as signed origin. +/// +/// Used for `claim_shielded_fees`: the validator calls the precompile from their +/// EVM address; their `H160` is mapped to an `AccountId` and used as the signed +/// origin so `ensure_signed` succeeds in the pallet. +pub fn from_caller( + handle: &mut impl PrecompileHandle, + call: pallet_shielded_pool::Call, +) -> PrecompileResult +where + T: pallet_evm::Config + pallet_shielded_pool::Config, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + RuntimeOriginOf: From::AccountId>>, + <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, + pallet_evm::AccountIdOf: Into<::AccountId>, +{ + let caller = handle.context().caller; + record_and_dispatch(handle, call, || { + let account: ::AccountId = + T::AddressMapping::into_account_id(caller).into(); + RuntimeOriginOf::::from(Some(account)) + }) +} + +/// Dispatches `call` with `None` origin (`ensure_none`). +/// +/// Used for `private_transfer` and `unshield`, where a ZK proof authenticates the +/// operation and no transaction signer is needed. +pub fn unsigned( + handle: &mut impl PrecompileHandle, + call: pallet_shielded_pool::Call, +) -> PrecompileResult +where + T: pallet_evm::Config + pallet_shielded_pool::Config, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + RuntimeOriginOf: From::AccountId>>, + <::RuntimeCall as Dispatchable>::PostInfo: core::fmt::Debug, +{ + record_and_dispatch(handle, call, || RuntimeOriginOf::::from(None)) +} diff --git a/ts-tests/node/abi-decoder-bounds.test.cjs b/ts-tests/node/abi-decoder-bounds.test.cjs new file mode 100644 index 00000000..11e7422c --- /dev/null +++ b/ts-tests/node/abi-decoder-bounds.test.cjs @@ -0,0 +1,190 @@ +// Adversarial probe of the shielded-pool precompile's ABI decoder. +// +// Everything the decoder reads is attacker-chosen calldata. Two bug classes were +// fixed and are attacked here: +// +// * Truncation. Offsets and lengths are `uint256`, but they were narrowed with +// `low_u32()`, which keeps the bottom 32 bits and discards the rest. A word +// of `2^32 + 8` read back as `8`, so the bounds check validated a value the +// sender never wrote and the decoder indexed somewhere else entirely. +// * Overflow. `data_start + length > params.len()` wraps on a huge length, and +// a wrapped sum passes the very check meant to reject it. Release builds do +// not enable overflow-checks, so it wrapped silently. +// +// Calldata is hand-built rather than encoded by a library: the point is to send +// shapes no encoder would produce. Every call must be refused without the node +// hanging, trapping, or stalling block production — so block height is checked +// after each batch. +// +// ./target/release/orbinum-node --dev --tmp --rpc-port 9955 --sealing=instant +// node ts-tests/node/abi-decoder-bounds.test.cjs +const { ApiPromise, WsProvider } = require('@polkadot/api'); +const { Keyring } = require('@polkadot/keyring'); + +const ok = [], bad = []; +const check = (n, c, x = '') => { (c ? ok : bad).push(n); console.log(`${c ? 'PASS' : 'FAIL'} ${n}${x ? ` — ${x}` : ''}`); }; +const sect = (t) => console.log(`\n── ${t} ──`); + +// Precompile index 2049. +const PRECOMPILE = '0x0000000000000000000000000000000000000801'; +const SHIELD_SELECTOR = '9feb22ea'; // shield(uint32,bytes32,bytes) +const RPC_TIMEOUT_MS = 15_000; + +/** 32-byte big-endian word from a BigInt. */ +const word = (v) => v.toString(16).padStart(64, '0'); + +/** eth_call against the precompile; classify the outcome. */ +async function ethCall(provider, data) { + const started = Date.now(); + try { + const result = await Promise.race([ + provider.send('eth_call', [{ to: PRECOMPILE, data: '0x' + data, gas: '0x100000' }, 'latest']), + new Promise((_, rej) => setTimeout(() => rej(new Error('TIMEOUT')), RPC_TIMEOUT_MS)), + ]); + return { outcome: 'ok', ms: Date.now() - started, result }; + } catch (e) { + if (e.message === 'TIMEOUT') return { outcome: 'timeout', ms: Date.now() - started }; + return { outcome: 'error', ms: Date.now() - started, message: e.message }; + } +} + +(async () => { + const provider = new WsProvider('ws://127.0.0.1:9955'); + const api = await ApiPromise.create({ provider, noInitWarn: true }); + const alice = new Keyring({ type: 'sr25519' }).addFromUri('//Alice'); + let nonce = (await api.rpc.system.accountNextIndex(alice.address)).toNumber(); + + const height = async () => (await api.rpc.chain.getHeader()).number.toNumber(); + const stillAlive = async (label) => { + const before = await height(); + await new Promise((res, rej) => { + const t = setTimeout(() => rej(new Error('timeout')), 30_000); + api.tx.system.remark('0x00').signAndSend(alice, { nonce: nonce++ }, ({ status }) => { + if (status.isInBlock) { clearTimeout(t); res(); } + }).catch(rej); + }); + check(`node still producing blocks after ${label}`, (await height()) > before); + }; + + sect('Truncated offsets'); + + // shield(uint32 asset_id, bytes32 commitment, bytes memo) + // Head: [asset_id][commitment][memo offset]. The memo offset is the word the + // decoder used to narrow with low_u32. + const head = (memoOffset) => + SHIELD_SELECTOR + word(0n) + 'aa'.repeat(32) + word(memoOffset); + + const truncating = [ + ['2^32 + 96 (low bits look like a valid offset)', (1n << 32n) + 96n], + ['2^32 exactly (truncates to 0)', 1n << 32n], + ['2^64 + 96', (1n << 64n) + 96n], + ['2^255 (top bit set)', 1n << 255n], + ['uint256 max', (1n << 256n) - 1n], + ]; + + for (const [label, offset] of truncating) { + // Pad out so a *truncated* offset would land inside the buffer — that is + // what made the old code accept these. + const data = head(offset) + word(4n) + 'ab'.repeat(4).padEnd(64, '0'); + const r = await ethCall(provider, data); + check(`memo offset ${label} is refused`, r.outcome === 'error', + `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`); + } + + await stillAlive('truncated offsets'); + + sect('Overflowing lengths'); + + // A valid offset pointing at a length word that would wrap the bounds check. + const overflowing = [ + ['usize::MAX', (1n << 64n) - 1n], + ['2^63 (wraps a 64-bit add)', 1n << 63n], + ['2^32 (wraps a 32-bit usize under Wasm)', 1n << 32n], + ['uint256 max', (1n << 256n) - 1n], + ]; + + for (const [label, length] of overflowing) { + const data = head(96n) + word(length) + '00'.repeat(32); + const r = await ethCall(provider, data); + check(`memo length ${label} is refused`, r.outcome === 'error', + `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`); + } + + await stillAlive('overflowing lengths'); + + sect('Truncated uint32'); + + // asset_id is declared uint32. A wider word used to keep its low bits, turning + // a malformed call into a plausible one naming a different asset. + for (const [label, assetId] of [ + ['2^32 + 0 (low bits = asset 0)', 1n << 32n], + ['2^32 + 1', (1n << 32n) + 1n], + ['uint256 max', (1n << 256n) - 1n], + ]) { + const data = SHIELD_SELECTOR + word(assetId) + 'aa'.repeat(32) + word(96n) + + word(4n) + 'ab'.repeat(4).padEnd(64, '0'); + const r = await ethCall(provider, data); + check(`asset_id ${label} is refused`, r.outcome === 'error', + `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`); + } + + await stillAlive('truncated uint32'); + + sect('Malformed shapes'); + + for (const [label, data] of [ + ['selector only', SHIELD_SELECTOR], + ['one byte short of a head', SHIELD_SELECTOR + '00'.repeat(95)], + ['offset pointing past the buffer', head(1_000_000n)], + ['offset pointing at itself', head(64n) + word(4n)], + ['empty calldata', ''], + ]) { + const r = await ethCall(provider, data); + check(`${label} is refused`, r.outcome === 'error', + `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`); + } + + await stillAlive('malformed shapes'); + + sect('Sustained pressure'); + + // Repeated rejections must not accumulate anywhere: the guards run before any + // allocation sized from calldata. + const burst = []; + for (let i = 0n; i < 40n; i++) { + burst.push(ethCall(provider, head((1n << 32n) + i) + word((1n << 63n) + i))); + } + const results = await Promise.all(burst); + const hung = results.filter((r) => r.outcome === 'timeout').length; + const accepted = results.filter((r) => r.outcome === 'ok').length; + const slowest = Math.max(...results.map((r) => r.ms)); + check('40 hostile calls, none hung', hung === 0, `slowest ${slowest}ms`); + check('40 hostile calls, none accepted', accepted === 0, `${accepted} returned ok`); + + await stillAlive('a 40-call burst'); + + sect('Well-formed calldata still works'); + + // The guards must not reject real calls, or they trade one bug for a worse one. + // A 180-byte memo is what the pallet requires. + const goodMemo = 'ab'.repeat(180).padEnd(384, '0'); // 180 bytes padded to 32-byte boundary + const good = SHIELD_SELECTOR + word(0n) + 'c1'.repeat(32) + word(96n) + + word(180n) + goodMemo; + const r = await ethCall(provider, good); + // eth_call with no value fails at the pallet's zero-amount check, not in the + // decoder — an ABI error would mean the guards broke valid encoding. + // "amount must be non-zero" comes from the precompile's own msg.value guard, + // which runs AFTER decoding — reaching it means the calldata decoded fine. + // An ABI-layer message here would mean the guards broke valid encoding. + const decoderRejected = r.outcome === 'error' && + /input too short|out of bounds|overflow|does not fit|exceeds u32|slot/i.test(r.message || ''); + check('well-formed calldata clears the decoder and reaches the pallet', + !decoderRejected, r.message ? r.message.slice(0, 70) : r.outcome); + + await stillAlive('a well-formed call'); + + await api.disconnect(); + console.log(`\n${ok.length} passed, ${bad.length} failed`); + if (bad.length) console.log('Failed: ' + bad.join(', ')); + process.exit(bad.length ? 1 : 0); +})().catch((e) => { console.error('ERROR:', e.message); process.exit(1); });