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 0000000000..acfd1a8a55 Binary files /dev/null and b/Libraries/LibWasm/Tests/Fixtures/Modules/return-mid-function.wasm differ 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 + ) +)