From 7caa897eaa339bb0fa0aec14c7bf5a306866448e Mon Sep 17 00:00:00 2001 From: Andrei Tumbar Date: Wed, 5 Aug 2026 07:29:26 -0700 Subject: [PATCH 1/4] Fix unchecked OOB & incorrect if-without-else --- crates/spacewasm_c_api/include/spacewasm.h | 1 + crates/spacewasm_c_api/src/status.rs | 2 ++ crates/spacewasm_c_api/src/tests.rs | 1 + src/error.rs | 1 + src/host.rs | 14 +++++--- src/interpreter.rs | 2 +- src/text.rs | 13 +++++-- src/types.rs | 19 +++++++--- src/util/rc.rs | 7 ++-- tests/regression/decode-errors.wast | 41 ++++++++++++++++++++++ tests/util/spectest.rs | 3 ++ 11 files changed, 89 insertions(+), 15 deletions(-) diff --git a/crates/spacewasm_c_api/include/spacewasm.h b/crates/spacewasm_c_api/include/spacewasm.h index 3e0dc13..c0da96a 100644 --- a/crates/spacewasm_c_api/include/spacewasm.h +++ b/crates/spacewasm_c_api/include/spacewasm.h @@ -72,6 +72,7 @@ enum spacewasm_status_t SPACEWASM_ERR_MEMORY_TOO_LARGE = 92, SPACEWASM_ERR_MEMORY_IMPORT_TOO_LARGE = 93, SPACEWASM_ERR_MEM_ALIGN_TOO_LARGE = 94, + SPACEWASM_ERR_TABLE_TOO_LARGE = 95, SPACEWASM_ERR_CONTROL_FLOW_TOO_DEEP = 96, SPACEWASM_ERR_STACK_UNDERFLOW = 97, SPACEWASM_ERR_STACK_TOO_LARGE = 98, diff --git a/crates/spacewasm_c_api/src/status.rs b/crates/spacewasm_c_api/src/status.rs index 6cbfe81..e3d2edf 100644 --- a/crates/spacewasm_c_api/src/status.rs +++ b/crates/spacewasm_c_api/src/status.rs @@ -72,6 +72,7 @@ pub enum spacewasm_status_t { SPACEWASM_ERR_MEMORY_TOO_LARGE = 92, SPACEWASM_ERR_MEMORY_IMPORT_TOO_LARGE = 93, SPACEWASM_ERR_MEM_ALIGN_TOO_LARGE = 94, + SPACEWASM_ERR_TABLE_TOO_LARGE = 95, // Parse / validation errors - Control flow validation SPACEWASM_ERR_CONTROL_FLOW_TOO_DEEP = 96, @@ -273,6 +274,7 @@ pub fn validation_status(e: &ValidationError) -> spacewasm_status_t { ValidationError::IdxTooLarge => SPACEWASM_ERR_IDX_TOO_LARGE, ValidationError::ModuleIdxTooLarge => SPACEWASM_ERR_MODULE_IDX_TOO_LARGE, ValidationError::MemoryTooLarge => SPACEWASM_ERR_MEMORY_TOO_LARGE, + ValidationError::TableTooLarge => SPACEWASM_ERR_TABLE_TOO_LARGE, ValidationError::MemoryImportTooLarge => SPACEWASM_ERR_MEMORY_IMPORT_TOO_LARGE, ValidationError::MemAlignTooLarge => SPACEWASM_ERR_MEM_ALIGN_TOO_LARGE, ValidationError::ControlFlowTooDeep => SPACEWASM_ERR_CONTROL_FLOW_TOO_DEEP, diff --git a/crates/spacewasm_c_api/src/tests.rs b/crates/spacewasm_c_api/src/tests.rs index 6260ee8..429871c 100644 --- a/crates/spacewasm_c_api/src/tests.rs +++ b/crates/spacewasm_c_api/src/tests.rs @@ -929,6 +929,7 @@ fn validation_error_codes_map() { status::SPACEWASM_ERR_MEMORY_IMPORT_TOO_LARGE, ), (MemAlignTooLarge, status::SPACEWASM_ERR_MEM_ALIGN_TOO_LARGE), + (TableTooLarge, status::SPACEWASM_ERR_TABLE_TOO_LARGE), // Control flow validation ( ControlFlowTooDeep, diff --git a/src/error.rs b/src/error.rs index ba0fea8..028a629 100644 --- a/src/error.rs +++ b/src/error.rs @@ -65,6 +65,7 @@ pub enum ValidationError { IdxTooLarge, ModuleIdxTooLarge, MemoryTooLarge, + TableTooLarge, MemoryImportTooLarge, MemAlignTooLarge, ControlFlowTooDeep, diff --git a/src/host.rs b/src/host.rs index 67448da..31a3ea8 100644 --- a/src/host.rs +++ b/src/host.rs @@ -164,8 +164,8 @@ pub enum HostFunctionBreak { pub type HostFunctionResult = ControlFlow>; -/// Maximum number of values in a host function parameter / result signature. -pub const HOST_SIGNATURE_CAP: usize = 63; +/// Maximum number of parameters a host function may declare. +pub const MAX_HOST_FUNCTION_PARAMS: usize = 9; /// Error returned when a host value signature contains an invalid character or /// exceeds [`HOST_SIGNATURE_CAP`] entries. @@ -177,7 +177,7 @@ pub struct HostValListError; /// `i` (i32), `I` (i64), `f` (f32), `d` (f64). #[derive(Copy, Clone)] pub struct HostValList { - data: [ValType; HOST_SIGNATURE_CAP], + data: [ValType; MAX_HOST_FUNCTION_PARAMS], len: u8, } @@ -215,11 +215,11 @@ impl HostValList { /// is not one of `iIfd` or the signature exceeds [`HOST_SIG_CAP`] entries. /// This is the FFI-safe constructor. pub fn try_new(s: &str) -> Result { - let mut data = [ValType::I32; HOST_SIGNATURE_CAP]; + let mut data = [ValType::I32; MAX_HOST_FUNCTION_PARAMS]; let mut len = 0usize; for c in s.chars() { - if len >= HOST_SIGNATURE_CAP { + if len >= MAX_HOST_FUNCTION_PARAMS { return Err(HostValListError); } data[len] = HostValList::map_char(c)?; @@ -373,6 +373,10 @@ impl HostFunction { return Err(HostValListError); } + if params.len() > MAX_HOST_FUNCTION_PARAMS { + return Err(HostValListError); + } + let mut rs: Option = None; for r in returns.iter() { if rs.is_some() { diff --git a/src/interpreter.rs b/src/interpreter.rs index 465d8ce..5a8521f 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -1384,7 +1384,7 @@ impl IrVisitor for Interpreter { x: u16, state: &mut Self::State, ) -> Result<(), Self::Error> { - let mut sv: StaticVec = StaticVec::new(); + let mut sv: StaticVec = StaticVec::new(); let f = &state.store.host_modules_mut()[module.0 as usize].functions[x as usize]; state.sp -= f.param_size(); diff --git a/src/text.rs b/src/text.rs index 4f257be..5552c53 100644 --- a/src/text.rs +++ b/src/text.rs @@ -582,9 +582,15 @@ impl<'a, const MAX_CONTROL_FRAMES: usize, const MAX_STACK_DEPTH: usize> // This bucket has the local variable // Compute it's offset as a word index from the frame let offset = current_offset + ty.size() * (x - current_index) as usize; + + let word_offset = offset / 4; + if word_offset > (i16::MAX as usize) - 2 { + return Err(ValidationError::LocalIdxOutOfRange); + } + return Ok(LocalVariable { // Add 2 to skip over fp and lr - frame_offset: ((offset / 4) as i16) + 2, + frame_offset: (word_offset as i16) + 2, ty: *ty, }); } @@ -1037,8 +1043,9 @@ impl<'a, const MAX_CONTROL_FRAMES: usize, const MAX_STACK_DEPTH: usize> } BlockKind::If => { // We are currently inside an if-statement without an else. - // Only if-statements without return values are valid here (or inside an unreachable state). - if last.out.0.is_none() || last.unreachable { + // A result-typed `if` requires an `else` because the false + // would diverge from the true path (different values returned) + if last.out.0.is_none() { let pc = self.pc(); self.code.backpatch(last.target, |code, address, label| { let patched = label.with_jump(JumpOffset::new(address, pc)?); diff --git a/src/types.rs b/src/types.rs index 4086394..f5e581a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -423,13 +423,24 @@ pub struct TableType { pub limits: Limit, } +/// Maximum number of elements permitted in a table. +pub const MAX_TABLE_ELEMENTS: u32 = 10_000_000; + impl TableType { pub(crate) fn read(wasm: &mut Reader) -> Result { // Table types are encoded with their limits and a constant byte indicating their element type. - Ok(TableType { - elem_type: ElemType::read(wasm)?, - limits: Limit::read(wasm)?, - }) + let elem_type = ElemType::read(wasm)?; + let limits = Limit::read(wasm)?; + + if limits.min > MAX_TABLE_ELEMENTS { + return Err(ValidationError::TableTooLarge); + } else if let Some(max) = limits.max { + if max > MAX_TABLE_ELEMENTS { + return Err(ValidationError::TableTooLarge); + } + } + + Ok(TableType { elem_type, limits }) } } diff --git a/src/util/rc.rs b/src/util/rc.rs index 215afeb..34a66b6 100644 --- a/src/util/rc.rs +++ b/src/util/rc.rs @@ -105,8 +105,11 @@ impl Rc<[T], A> { // Calculate the layout we need: Cell + align padding + [T; len] let count_layout = core::alloc::Layout::new::>(); - let slice_layout = core::alloc::Layout::array::(len).unwrap(); - let (full_layout, slice_offset) = count_layout.extend(slice_layout).unwrap(); + let slice_layout = + core::alloc::Layout::array::(len).map_err(|_| AllocError::OutOfMemory)?; + let (full_layout, slice_offset) = count_layout + .extend(slice_layout) + .map_err(|_| AllocError::OutOfMemory)?; let full_layout = full_layout.pad_to_align(); // Allocate new memory for RcInner<[T]> diff --git a/tests/regression/decode-errors.wast b/tests/regression/decode-errors.wast index c4a5d0d..edafb71 100644 --- a/tests/regression/decode-errors.wast +++ b/tests/regression/decode-errors.wast @@ -38,6 +38,17 @@ (module binary "\00asm\01\00\00\00\04\05\01\70\01\02\01") "size minimum must not be greater than maximum") +;; --------------------------------------------------------------------------- +;; A table whose `limits.min` is used unbounded as an allocation length is +;; rejected at decode time (symmetric with the memory-size bound). Left +;; unchecked, `min` drove a panic on 32-bit targets (Layout::array failure) or +;; a multi-gigabyte allocation on 64-bit hosts. +;; table section (id 4): count=1, funcref, flag=0x00 (min only), min=0xFFFFFFFF +;; --------------------------------------------------------------------------- +(assert_invalid + (module binary "\00asm\01\00\00\00\04\08\01\70\00\ff\ff\ff\ff\0f") + "table size too large") + ;; --------------------------------------------------------------------------- ;; Memory type flag with the "shared" bit (bit 1) set is unsupported. ;; memory section (id 5): count=1, flag 0x02, min 0 @@ -109,3 +120,33 @@ (assert_malformed (module binary "\00asm\01\00\00\00\0c\01\00") "malformed section id") + +;; --------------------------------------------------------------------------- +;; A local variable's frame offset is encoded as a signed 16-bit value, but the +;; validator otherwise permits up to 0xFFFF words of locals. A high local index +;; used to wrap the `as i16` cast into a negative offset, producing an +;; out-of-bounds stack read/write at runtime. The offset-encoding site now +;; rejects any local whose word offset cannot be represented. +;; One function declaring 40000 i32 locals (accepted by the size validator) +;; whose body accesses `local.get 35000` (word offset 35000 > i16::MAX - 2). +;; --------------------------------------------------------------------------- +(assert_invalid + (module binary + "\00asm\01\00\00\00\01\04\01\60\00\00\03\02\01\00\07\08" + "\01\04\74\65\73\74\00\00\0a\0d\01\0b\01\c0\b8\02\7f\20\b8\91" + "\02\1a\0b") + "local offset out of range") + +;; --------------------------------------------------------------------------- +;; A result-typed `if` without an `else` used to be accepted when the then-arm +;; ended unreachable. The false path is still reachable and produces no result, +;; desynchronizing the validator's operand-stack model from the runtime stack +;; pointer. Such a module must be rejected regardless of then-arm reachability. +;; Function `f` of type () -> i32 whose body is +;; `i32.const 0; if (result i32); unreachable; end`. +;; --------------------------------------------------------------------------- +(assert_invalid + (module binary + "\00asm\01\00\00\00\01\05\01\60\00\01\7f\03\02\01\00\07" + "\05\01\01\66\00\00\0a\0a\01\08\00\41\00\04\7f\00\0b\0b") + "result-typed if without else") diff --git a/tests/util/spectest.rs b/tests/util/spectest.rs index c49a990..4f02ace 100644 --- a/tests/util/spectest.rs +++ b/tests/util/spectest.rs @@ -982,6 +982,9 @@ fn check_decode_error(err: ParseError, text: String) { (ValidationError::InvalidMaxLimit, "size minimum must not be greater than maximum") => {} (ValidationError::MemoryTooLarge, "memory size must be at most 65536 pages (4GiB)") => {} (ValidationError::MemoryTooLarge, "memory size must be at most 4 GiB") => {} + (ValidationError::TableTooLarge, "table size too large") => {} + (ValidationError::LocalIdxOutOfRange, "local offset out of range") => {} + (ValidationError::BlockResultTypeMismatch, "result-typed if without else") => {} (ValidationError::InvalidNegativeMemOffset, "data segment does not fit") => {} (ValidationError::InvalidMemOffsetType, "type mismatch") => {} (ValidationError::InvalidStartFunctionSignature, "start function") => {} From 05ccfb80d8ac34a012e1937995daa97fd80c9a94 Mon Sep 17 00:00:00 2001 From: Andrei Tumbar Date: Wed, 5 Aug 2026 07:40:21 -0700 Subject: [PATCH 2/4] Clean up comment --- src/text.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/text.rs b/src/text.rs index 5552c53..6826049 100644 --- a/src/text.rs +++ b/src/text.rs @@ -1043,8 +1043,7 @@ impl<'a, const MAX_CONTROL_FRAMES: usize, const MAX_STACK_DEPTH: usize> } BlockKind::If => { // We are currently inside an if-statement without an else. - // A result-typed `if` requires an `else` because the false - // would diverge from the true path (different values returned) + // Only if-statements without return values are valid here if last.out.0.is_none() { let pc = self.pc(); self.code.backpatch(last.target, |code, address, label| { From 79e9c318af91c4946d570f5d1afe06f03d5c6d9f Mon Sep 17 00:00:00 2001 From: Andrei Tumbar Date: Wed, 5 Aug 2026 07:48:40 -0700 Subject: [PATCH 3/4] Convert test to text instead of bin --- tests/regression/decode-errors.wast | 19 +++++++++---------- tests/regression_integration.rs | 2 -- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/regression/decode-errors.wast b/tests/regression/decode-errors.wast index edafb71..f0da065 100644 --- a/tests/regression/decode-errors.wast +++ b/tests/regression/decode-errors.wast @@ -122,13 +122,8 @@ "malformed section id") ;; --------------------------------------------------------------------------- -;; A local variable's frame offset is encoded as a signed 16-bit value, but the -;; validator otherwise permits up to 0xFFFF words of locals. A high local index -;; used to wrap the `as i16` cast into a negative offset, producing an -;; out-of-bounds stack read/write at runtime. The offset-encoding site now -;; rejects any local whose word offset cannot be represented. -;; One function declaring 40000 i32 locals (accepted by the size validator) -;; whose body accesses `local.get 35000` (word offset 35000 > i16::MAX - 2). +;; We normally accept up to 0xFFFF locals but the real check if whether it's +;; 16-bit word offset overflows i16::MAX. This is a failure case. ;; --------------------------------------------------------------------------- (assert_invalid (module binary @@ -146,7 +141,11 @@ ;; `i32.const 0; if (result i32); unreachable; end`. ;; --------------------------------------------------------------------------- (assert_invalid - (module binary - "\00asm\01\00\00\00\01\05\01\60\00\01\7f\03\02\01\00\07" - "\05\01\01\66\00\00\0a\0a\01\08\00\41\00\04\7f\00\0b\0b") + (module + (type $t0 (func (result i32))) + (func $f (export "f") (type $t0) (result i32) + (if $I0 (result i32) + (i32.const 0) + (then + (unreachable))))) "result-typed if without else") diff --git a/tests/regression_integration.rs b/tests/regression_integration.rs index a9a4238..0abf273 100644 --- a/tests/regression_integration.rs +++ b/tests/regression_integration.rs @@ -1,5 +1,3 @@ -#![cfg(not(miri))] - mod util; use std::{ops::ControlFlow, sync::Mutex}; From 3066770a051badc7b4de9176aac187d776a30dc0 Mon Sep 17 00:00:00 2001 From: Andrei Tumbar Date: Wed, 5 Aug 2026 07:53:13 -0700 Subject: [PATCH 4/4] Disable miri on integration test --- tests/regression_integration.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/regression_integration.rs b/tests/regression_integration.rs index 0abf273..a9a4238 100644 --- a/tests/regression_integration.rs +++ b/tests/regression_integration.rs @@ -1,3 +1,5 @@ +#![cfg(not(miri))] + mod util; use std::{ops::ControlFlow, sync::Mutex};