Skip to content

Fix use-after-free in AggregateError creation for multi-error module loads#31399

Closed
robobun wants to merge 1 commit into
mainfrom
farm/e1ad33a3/fix-buildmessage-aggregate-uaf
Closed

Fix use-after-free in AggregateError creation for multi-error module loads#31399
robobun wants to merge 1 commit into
mainfrom
farm/e1ad33a3/fix-buildmessage-aggregate-uaf

Keep multi-error AggregateError children GC-visible while they are cr…

2f3bf67
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 25, 2026 in 14m 34s

Code review found 2 potential issues

Found 2 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 1
Severity File:Line Issue
🟡 Nit test/js/bun/resolve/build-error.test.ts:69 Test asserts absence of 'AddressSanitizer' in stderr — prohibited pattern
🟣 Pre-existing src/jsc/VirtualMachine.rs:2561-2569 Same heap-Vec UAF pattern remains in LogJsc::to_js — fix is incomplete

Annotations

Check warning on line 69 in test/js/bun/resolve/build-error.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Test asserts absence of 'AddressSanitizer' in stderr — prohibited pattern

nit: `expect(stderr).not.toContain("AddressSanitizer")` is the kind of assertion CLAUDE.md says to avoid ("NEVER write tests that check for no 'panic' or 'uncaught exception' or similar in the test output. These tests will never fail in CI."). Non-ASAN CI builds will never emit this string, and on ASAN builds a UAF aborts the subprocess so `expect(exitCode).toBe(0)` and `expect(stdout).toContain("OK 250")` already catch it — this line can be dropped.

Check notice on line 2569 in src/jsc/VirtualMachine.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Same heap-Vec UAF pattern remains in LogJsc::to_js — fix is incomplete

Heads-up: the same heap-`Vec<JSValue>` UAF pattern this PR fixes still exists in `LogJsc::to_js` at `src/jsc/lib.rs:2146-2151` — `let mut errors_stack: Vec<JSValue> = …` followed by a loop pushing `msg_to_js()` (which calls the same `BuildMessage::create`/`ResolveMessage::create`). That path is live via `Bun.Transpiler` (`scanImports`/`transformSync`), `web_worker`, `TOMLObject`, `JSONCObject`, `css_internals`, and `filesystem_router` for any input with 2+ log messages. This is pre-existing code