LibWasm: Fix “return” leaking intermediate value-stack entries to caller
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
This commit is contained in:
parent
1a9c7f564b
commit
59eb221d7c
4 changed files with 79 additions and 0 deletions
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
14
Libraries/LibWasm/Tests/Executor/test-return-mid-function.js
Normal file
14
Libraries/LibWasm/Tests/Executor/test-return-mid-function.js
Normal file
|
|
@ -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);
|
||||
});
|
||||
Binary file not shown.
|
|
@ -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
|
||||
)
|
||||
)
|
||||
Loading…
Reference in a new issue