From 80dbc50b08741a4168f41fdea8185b9539520fd0 Mon Sep 17 00:00:00 2001
From: 94xhn <87560781+94xhn@users.noreply.github.com>
Date: Thu, 16 Jul 2026 13:23:07 +0800
Subject: [PATCH] Fix integer-overflow bypass of bounds check in get_safeptr()

get_safeptr() is the bounds-checking primitive behind
uvm32_arg_getslice()/uvm32_arg_getslice_fixed(), used by a host to safely
turn a (ptr, len) syscall argument pair supplied by VM bytecode into a
slice. Both addr and len come directly from VM registers a0/a1, so are
fully attacker-controlled if the running bytecode is untrusted.

The guard computed ptrstart + len in uint32_t and compared the sum
against the memory limit. If len is chosen so that ptrstart + len wraps
past 2^32 (e.g. ptrstart=1, len=0xFFFFFFFF), the wrapped sum can land
back at or below the limit, bypassing the check while returning a slice
whose declared len vastly exceeds the real buffer. Any host code that
subsequently trusts that len for a copy/read/write will overrun its
buffer.

Fix by comparing len against (limit - ptrstart) after confirming
ptrstart <= limit, which cannot overflow. Applies to both the main
memory path and the extram path.

Added a regression test (test_syscall_args_bufrd_overflow_bypass) that
requests a slice with a real in-bounds pointer but len=0xFFFFFFFF and
asserts it is now rejected.
---
 test/syscall_args/test/tests.c | 26 ++++++++++++++++++++++++++
 uvm32/uvm32.c                  |  8 ++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/test/syscall_args/test/tests.c b/test/syscall_args/test/tests.c
index 03836a9..882d813 100644
--- a/test/syscall_args/test/tests.c
+++ b/test/syscall_args/test/tests.c
@@ -211,6 +211,32 @@ void test_syscall_args_bufrd_toolarge(void) {
     TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD);
 }
 
+void test_syscall_args_bufrd_overflow_bypass(void) {
+    // run the vm
+    uvm32_run(&vmst, &evt, 1000);
+    // check for picktest syscall
+    TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL);
+    TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST);
+    uvm32_arg_setval(&vmst, &evt, RET, SYSCALL_C);
+
+    uvm32_run(&vmst, &evt, 1000);
+    TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL);
+    TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_C);
+
+    // ARG0 is a real, in-bounds pointer supplied by the rom. Simulate bytecode
+    // supplying an attacker-controlled huge length in ARG1: previously
+    // (ptrstart + len) wrapped past 2^32 back under the memory limit inside
+    // get_safeptr(), bypassing the bounds check entirely.
+    uvm32_arg_setval(&vmst, &evt, ARG1, 0xFFFFFFFF);
+
+    uvm32_slice_t buf = uvm32_arg_getslice(&vmst, &evt, ARG0, ARG1);
+    TEST_ASSERT_EQUAL(0, buf.len);  // must be safely rejected, not silently overflowed
+    // attempt to run vm, should be stuck in err
+    uvm32_run(&vmst, &evt, 100);
+    TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_ERR);
+    TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD);
+}
+
 void test_syscall_args_read_badram(void) {
     // run the vm
     uvm32_run(&vmst, &evt, 1000);
diff --git a/uvm32/uvm32.c b/uvm32/uvm32.c
index ba4f779..d5e629b 100644
--- a/uvm32/uvm32.c
+++ b/uvm32/uvm32.c
@@ -190,7 +190,9 @@ static bool get_safeptr(uvm32_state_t *vmst, uint32_t addr, uint32_t len, uvm32_
             return false;
         } else {
             uint32_t ptrstart = addr - UVM32_EXTRAM_BASE;
-            if ((ptrstart > vmst->_extramLen) || (ptrstart + len > vmst->_extramLen)) {
+            // Compare against (limit - ptrstart) rather than (ptrstart + len) to avoid
+            // len wrapping ptrstart + len back below the limit on unsigned overflow.
+            if ((ptrstart > vmst->_extramLen) || (len > vmst->_extramLen - ptrstart)) {
                 setStatusErr(vmst, UVM32_ERR_MEM_RD);
                 buf->ptr = (uint8_t *)NULL;
                 buf->len = 0;
@@ -202,7 +204,9 @@ static bool get_safeptr(uvm32_state_t *vmst, uint32_t addr, uint32_t len, uvm32_
         }
     } else {
         uint32_t ptrstart = addr - MINIRV32_RAM_IMAGE_OFFSET;
-        if ((ptrstart > UVM32_MEMORY_SIZE) || (ptrstart + len > UVM32_MEMORY_SIZE)) {
+        // Compare against (limit - ptrstart) rather than (ptrstart + len) to avoid
+        // len wrapping ptrstart + len back below the limit on unsigned overflow.
+        if ((ptrstart > UVM32_MEMORY_SIZE) || (len > UVM32_MEMORY_SIZE - ptrstart)) {
             setStatusErr(vmst, UVM32_ERR_MEM_RD);
             buf->ptr = (uint8_t *)NULL;
             buf->len = 0;