Skip to content

bug(wasm): Arithmetic overflow in MemoryRegion::contains bounds checking #1303

@AftAb-25

Description

@AftAb-25

🔍 Issue Description

📌 Issue Type

  • Bug
  • Feature Request
  • Enhancement
  • Documentation
  • Refactor
  • Other (please specify)

📝 Description

There are arithmetic overflows in the WASM memory management module (crates/mofa-plugins/src/wasm_runtime/memory.rs), specifically due to the use of unchecked + and * operations on u32 values.

What is happening?

  1. In MemoryRegion::contains, the bounds check uses addr.0 < self.start.0 + self.size. If the region sits near the end of the 32-bit address space, start.0 + size will overflow. In release builds, this silently wraps around to a small number, causing valid addresses within the region to fail the check, while low memory addresses are incorrectly identified as being inside this high memory region. In debug builds, this panics with attempt to add with overflow.
  2. In WasmMemory::size, the total byte size is calculated as self.pages * Self::PAGE_SIZE. Since PAGE_SIZE is 65,536, any pages value ≥ 65,536 will overflow the u32 result.
  3. In MemoryAllocator::return_block, when coalescing blocks, the combined size is calculated as block.size + coalesced.size, which can overflow if merging large blocks.

What should happen instead?
Mathematical operations involving WASM memory boundaries and sizes should be immune to overflow wrappers. Inequalities should be rewritten to avoid addition, and multiplication/addition of sizes should use checked_add/checked_mul or upcast to u64.

Why is this needed?
To ensure strict memory bounds checking within the WASM sandbox and prevent silent logic corruption or unexpected panics in the runtime.


🎯 Proposed Solution (Optional but Encouraged)

High-level approach & Relevant files:
Modify crates/mofa-plugins/src/wasm_runtime/memory.rs to fix the math operations:

  1. Fix MemoryRegion::contains:
    Rewrite the inequality to subtract instead of add:
pub fn contains(&self, addr: GuestPtr) -> bool {
    addr.0 >= self.start.0 && addr.0 - self.start.0 < self.size
}
  1. Fix MemoryAllocator::return_block: Use checked_add or saturation:
rust
coalesced = MemoryRegion::new(block.start, block.size.saturating_add(coalesced.size));
  1. Fix WasmMemory::size: Change the method to either return a u64, or at least use saturating_mul if we accept clamping at u32::MAX:
rust
pub fn size(&self) -> u32 {
    self.pages.saturating_mul(Self::PAGE_SIZE)
}

📎 Additional Context

  • File: crates/mofa-plugins/src/wasm_runtime/memory.rs
  • Lines affected: 102 (contains), 140 (size), 436/440 (return_block).
  • If start is 0xFFFF_FFF0 and size is 32, start + size correctly mathematical represents 0x1_0000_0010, but in u32 it wraps to 0x10. Calling .contains(GuestPtr::new(5)) will incorrectly return true because 5 < 0x10.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/platformInstallation, OS integration, environmentkind/bugSomething is brokenpriority/p1High impactrustPull requests that update rust codesecuritystatus/manual-triageRequires manual review

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions