From 59eb221d7cd95a466af34fd558f8b496abc24a46 Mon Sep 17 00:00:00 2001
From: sideshowbarker <mike@w3.org>
Date: Mon, 25 May 2026 14:58:24 +0900
Subject: [PATCH] =?UTF-8?q?LibWasm:=20Fix=20=E2=80=9Creturn=E2=80=9D=20lea?=
 =?UTF-8?q?king=20intermediate=20value-stack=20entries=20to=20caller?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem: Loading WebAssembly modules that use “return” mid-function can
corrupt the heap and crash the browser.

Cause: HANDLE_INSTRUCTION(return_) shrank the label stack but left any
working values pushed before the “return” instruction on the shared
value stack. Those residuals leaked into the caller’s frame and
accumulated across calls — until they overflowed the value stack’s
inline storage and corrupted adjacent allocator metadata.

Fix: After shrinking the label stack down to the function-level label,
also remove value-stack entries between that label’s recorded
stack_height and the top .arity() result values — mirroring the cleanup
that branch_to_label<true> already performs for br/br_if.

Fixes https://github.com/LadybirdBrowser/ladybird/issues/9614
---
 .../AbstractMachine/BytecodeInterpreter.cpp   |   7 +++
 .../Executor/test-return-mid-function.js      |  14 +++++
 .../Fixtures/Modules/return-mid-function.wasm | Bin 0 -> 106 bytes
 .../Fixtures/Modules/return-mid-function.wat  |  58 ++++++++++++++++++
 4 files changed, 79 insertions(+)
 create mode 100644 Libraries/LibWasm/Tests/Executor/test-return-mid-function.js
 create mode 100644 Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wasm
 create mode 100644 Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wat

diff --git a/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp b/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp
index 0e87d5797a..374b7aaa2f 100644
--- a/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp
+++ b/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp
@@ -2131,6 +2131,13 @@ HANDLE_INSTRUCTION(return_)
 {
     LOG_INSN;
     configuration.label_stack().shrink(configuration.frame().label_index() + 1, true);
+    // Clear intermediate working values from the value stack, keeping only the top .arity() (the return values) above
+    // the function-level label's recorded stack_height. Without this, residual values pushed before the return are
+    // leaked to the caller’s value stack — and accumulate across nested calls until heap-buffer-overflow.
+    auto const& label = configuration.label_stack().unsafe_last();
+    auto& vs = configuration.value_stack();
+    if (vs.size() > label.stack_height() + label.arity())
+        vs.remove(label.stack_height(), vs.size() - label.stack_height() - label.arity());
     return Outcome::Return;
 }
 
diff --git a/Libraries/LibWasm/Tests/Executor/test-return-mid-function.js b/Libraries/LibWasm/Tests/Executor/test-return-mid-function.js
new file mode 100644
index 0000000000..a20275f796
--- /dev/null
+++ b/Libraries/LibWasm/Tests/Executor/test-return-mid-function.js
@@ -0,0 +1,14 @@
+test("return mid-function doesn't corrupt caller stack", () => {
+    const bin = readBinaryWasmFile("Fixtures/Modules/return-mid-function.wasm");
+    const module = parseWebAssemblyModule(bin);
+
+    // Deterministic check: without the fix, the residual 99 from $leaky sits between the caller’s 10 and the result 42,
+    // so i32.add yields 141, rather than the correct 52.
+    const test_add = module.getExport("test_add");
+    expect(module.invoke(test_add)).toBe(52);
+
+    // Stress check: 100 iterations accumulate residuals under the bug; under ASan this overflows the value stack’s
+    // inline storage.
+    const drive = module.getExport("drive");
+    expect(module.invoke(drive)).toBe(100 * 42);
+});
diff --git a/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wasm b/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wasm
new file mode 100644
index 0000000000000000000000000000000000000000..acfd1a8a5541e529817087d481de789b888cc500
GIT binary patch
literal 106
zcmZQbEY4+QU|?WmWlUgTtY>Cr0dg4FMVL5BQj1IC6H`)vQYl55WvL9zTsABm433W(
z9JTnl*%=(U1Q@coxfvX<b2zRO1c}KqGSxFX2rwxyW+^f%FgP-1DKR)cVesH(;8tMd
F1^}$D5r_Z)

literal 0
HcmV?d00001

diff --git a/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wat b/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wat
new file mode 100644
index 0000000000..0d21b93c1f
--- /dev/null
+++ b/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wat
@@ -0,0 +1,58 @@
+(module
+  ;; Regression test for https://github.com/LadybirdBrowser/ladybird/issues/9614
+  ;;
+  ;; The 'return' instruction must drop intermediate working values from the value stack, keeping only the top .arity()
+  ;; values (the return values). The spec validation rule for 'return' doesn’t require the value stack to be otherwise
+  ;; empty when return fires. So a function is allowed to leave extras on the stack, provided the top items match the
+  ;; result types.
+  ;;
+  ;; Before the fix for #9614, the interpreter's return_ handler only shrank the label stack and left residual values on
+  ;; the shared value stack, leaking them to the caller.
+
+  ;; Pushes residual 99, then returns 42. The residual must be discarded.
+  (func $leaky (result i32)
+    i32.const 99
+    i32.const 42
+    return
+  )
+
+  ;; Deterministic correctness check: caller pushes 10, calls $leaky, adds. With the fix: value stack after
+  ;; call = [10, 42]; i32.add yields 52. Without the fix: residual 99 sits between 10 and 42. So, i32.add consumes
+  ;; 99+42=141. Wrong answer, caught immediately without ASan.
+  (func (export "test_add") (result i32)
+    i32.const 10
+    call $leaky
+    i32.add
+  )
+
+  ;; Pushes three values, then returns the top one. The bottom two are residuals; 'return' is required to discard them.
+  (func $return_with_residuals (result i32)
+    i32.const 1111
+    i32.const 2222
+    i32.const 42
+    return
+  )
+
+  ;; Calls $return_with_residuals 100 times. Without the fix for #9614, each call leaks two residuals into the driver's
+  ;; value stack. Under ASan, that eventually overflows the value stack's inline storage as heap-buffer-overflow. With
+  ;; the fix, every iteration is net-zero and acc == 100 * 42.
+  (func (export "drive") (result i32)
+    (local $i i32)
+    (local $acc i32)
+    (loop $loop
+      call $return_with_residuals
+      local.get $acc
+      i32.add
+      local.set $acc
+
+      local.get $i
+      i32.const 1
+      i32.add
+      local.tee $i
+      i32.const 100
+      i32.lt_s
+      br_if $loop
+    )
+    local.get $acc
+  )
+)