Fix use-after-free in Bun.Transpiler.transform() error reporting by robobun · Pull Request #30309 · oven-sh/bun

robobun · 2026-05-06T05:34:15Z

What does this PR do?

Fixes a use-after-free when Bun.Transpiler#transform() (the async variant) rejects with parse errors.

TransformTask.run() runs on a worker thread and uses a MimallocArena for parsing. Any parser/lexer diagnostics added to this.log have their text (msg.data.text, notes) allocated from that arena. The arena is destroyed at the end of run(), but the log messages are read later on the JS thread in then() via this.log.toJS() → BuildMessage.create() → Msg.clone() → allocator.dupe(u8, this.text), which reads freed arena memory.

Setting this.log.msgs.allocator = bun.default_allocator only keeps the Msg array alive; the text inside each message still lives in the arena.

The fix clones each log message into bun.default_allocator in a defer that runs before arena.deinit(), so the message contents survive until then() runs.

How did you verify your code works?

Added a regression test that runs 20 concurrent transform() calls on source with many parse errors. Under ASAN without the fix this reliably triggers use-after-poison at the same PC as the fuzzer report; with the fix it passes cleanly and the error messages/positions are intact.

test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts

All existing test/js/bun/transpiler/ and test/bundler/transpiler/transpiler.test.js tests continue to pass.

TransformTask.run() uses a MimallocArena for parsing that is destroyed when run() returns. Parser/lexer error messages have their text allocated in that arena, so when then() later calls log.toJS() on the JS thread and clones the messages, it reads freed memory. Clone the log messages into bun.default_allocator before the arena is destroyed so they survive until then() runs.

robobun · 2026-05-06T05:34:25Z

^{Updated 4:58 AM PT - May 6th, 2026}

❌ @robobun, your commit ba94fd0 has 2 failures in Build #52066 (All Failures):

🧪 To try this PR locally:

bunx bun-pr 30309

That installs a local version of the PR into your bun-30309 executable, so you can run:

bun-30309 --bun

coderabbitai · 2026-05-06T05:37:10Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

TransformTask now clones log messages during run and frees cloned location strings during deinit via a new helper to avoid use-after-free; a new concurrent test spawns 20 invalid transpile tasks and asserts each returns a parse-related Error.

Changes

Transpiler log message reallocation + concurrent error UAF test

Layer / File(s)	Summary
Memory-handling helper `src/runtime/api/JSTranspiler.zig`	Added `fn freeClonedLocation(data: *logger.Data)` to free cloned `location.file` and `location.line_text` when present.
Run-time cloning `src/runtime/api/JSTranspiler.zig`	In `TransformTask.run` added a `defer` that iterates `this.log.msgs` and clones each message with `msg.clone(bun.default_allocator)` guarded by `bun.handleOom`.
Deinitialization cleanup `src/runtime/api/JSTranspiler.zig`	In `TransformTask.deinit` added logic to call `freeClonedLocation` for each message and its notes, then deinitialize messages.
Tests `test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts`	New test: builds a large invalid JS string, constructs a `Bun.Transpiler`, runs 20 concurrent `transpiler.transform` invocations via `Promise.all`, asserts each result is an `Error` with message matching `/already been declared

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main fix: addressing a use-after-free bug in Bun.Transpiler.transform() error reporting, which aligns with the core objective of the changeset.
Description check	✅ Passed	The PR description fully covers both required template sections with clear, detailed explanations of what the fix does and how it was verified with a regression test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-05-06T05:39:25Z

This PR may be a duplicate of:

Fix use-after-free in Bun.Transpiler async transform() errors #29958 - Fixes the same async transform() log message use-after-free with a different approach (separate arena-backed log + cloneArenaLog with StringBuilder)
JSTranspiler: fix use-after-free of log messages in async transform() #30180 - Fixes the same async transform() log message use-after-free using log.cloneToWithRecycled() to deep-copy the log before arena teardown
Fix use-after-free in Bun.Transpiler async transform() errors #30263 - Fixes the same async transform() log message use-after-free with a nearly identical defer block cloning messages via msg.clone(bun.default_allocator)

🤖 Generated with Claude Code

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts`:
- Around line 2-23: The test writes a one-off script to "index.js" then calls
Bun.spawnSync with cmd [bunExe(), "run", "index.js"]; replace that by invoking
bunExe() with the -e flag and passing the script body directly (i.e. use
bunExe(), "-e", "<script>") so this single-file subprocess test follows the
guideline; update the Bun.spawnSync cmd array accordingly (and you can remove
the temp file creation via tempDirWithFiles or keep the temp dir for other uses
if needed), referencing bunExe(), Bun.spawnSync, and tempDirWithFiles in the
change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9b0e2509-0d7d-4294-b2d0-3e383dfd9c2a

📥 Commits

Reviewing files that changed from the base of the PR and between 8661224 and 5eada11.

📒 Files selected for processing (1)

test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts

claude

Both prior concerns are addressed and the clone/free pairing now looks correct to me; leaving the final call to a maintainer since this is cross-thread allocator ownership and there are three competing PRs (#29958/#30180/#30263) taking different approaches to the same UAF.

Extended reasoning...

Overview

This PR fixes a use-after-free in Bun.Transpiler#transform() (async variant) where parser diagnostics allocated in a worker-thread MimallocArena were read on the JS thread after the arena was destroyed. The fix adds a defer in TransformTask.run() that deep-clones each Msg into bun.default_allocator before arena.deinit(), and adds matching cleanup in TransformTask.deinit() (including a freeClonedLocation helper to compensate for Location.deinit being a no-op). A subprocess regression test is added.

Security risks

None. This is internal memory-lifetime management for diagnostic strings; no auth, crypto, or untrusted-input parsing changes beyond what already existed.

Level of scrutiny

Medium-high. The change is small (~20 lines) but touches manual allocator ownership across a thread boundary in runtime code. I verified against src/logger/logger.zig that Msg.clone deep-clones data.text, location.file, location.line_text, and recursively each note via bun.clone, and that the new deinit path frees exactly those allocations (freeClonedLocation for location strings since Location.deinit is a no-op at logger.zig:149, then msg.deinit for data.text, note texts, and the notes slice). Zero-length text/file slices are safe to free. Defer ordering is correct (clone runs before arena.deinit()).

Other factors

Both issues I raised earlier (the introduced leak, and the explicit test timeout) have been addressed. The remaining reason I'm not auto-approving is not a correctness concern but a merge-strategy one: github-actions already flagged three other open PRs fixing the same bug with different mechanisms (separate arena + StringBuilder, log.cloneToWithRecycled, and a near-identical defer-clone). A maintainer should decide which approach to land — and whether the freeClonedLocation workaround here is preferable to fixing Location.deinit itself. CI failures on this PR (Windows http/hot/jsc-stress) are unrelated to the change.

github-actions Bot added the claude label May 6, 2026

[autofix.ci] apply automated fixes

f64ce52

claude Bot reviewed May 6, 2026

View reviewed changes

Comment thread src/runtime/api/JSTranspiler.zig

robobun added 2 commits May 6, 2026 05:58

Free cloned log messages in TransformTask.deinit()

8661224

Run regression test in subprocess so crash registers as test failure

5eada11

coderabbitai Bot reviewed May 6, 2026

View reviewed changes

Comment thread test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts Outdated

Use bun -e for subprocess regression test

22c4a7b

claude Bot reviewed May 6, 2026

View reviewed changes

Comment thread test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts Outdated

robobun mentioned this pull request May 6, 2026

Add support for cwd argument in bunx #28668

Open

Drop explicit test timeout

ba94fd0

claude Bot reviewed May 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix use-after-free in Bun.Transpiler.transform() error reporting#30309

Fix use-after-free in Bun.Transpiler.transform() error reporting#30309
robobun wants to merge 6 commits into
mainfrom
farm/ee469c7e/transpiler-transform-log-uaf

robobun commented May 6, 2026

Uh oh!

robobun commented May 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 6, 2026 •

edited

Loading

Reviews paused

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented May 6, 2026

What does this PR do?

How did you verify your code works?

Uh oh!

robobun commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented May 6, 2026 •

edited

Loading

coderabbitai Bot commented May 6, 2026 •

edited

Loading