Skip to content

Fix use-after-free in Bun.Transpiler async transform() parse errors #30020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
Closed

Fix use-after-free in Bun.Transpiler async transform() parse errors #30020

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efe71ac
Fix use-after-free in Bun.Transpiler async transform() parse errors
robobun Apr 30, 2026
daaf253
[autofix.ci] apply automated fixes
autofix-ci[bot] Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/bun.js/api/JSTranspiler.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,16 @@
var arena = MimallocArena.init();
defer arena.deinit();

// Error messages added by the parser/lexer may contain text allocated
// in the arena. Deep-clone them into default_allocator before the arena
// is freed so they remain valid when then() reads them on the JS thread.
defer if (this.log.msgs.items.len > 0) {
var new_log = logger.Log.init(bun.default_allocator);
new_log.level = this.log.level;
bun.handleOom(this.log.appendToWithRecycled(&new_log, true));
this.log = new_log;
};

Check warning on line 502 in src/bun.js/api/JSTranspiler.zig

View check run for this annotation

Claude / Claude Code Review

Memory leak: cloned log message buffers never freed 

Minor: `appendToWithRecycled` allocates a `StringBuilder` buffer and a `notes_buf` in `bun.default_allocator` to hold the cloned message text, but `Log.deinit()` only does `msgs.clearAndFree()` and never frees those buffers — so each async `transform()` parse error leaks the cloned error text. This is the same pre-existing pattern as every other `appendToWithRecycled` caller and trading a UAF for a small error-path leak is clearly the right call here; just flagging it as a candidate for a follow
Comment on lines +494 to +502

@claude claude Bot May 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor: appendToWithRecycled allocates a StringBuilder buffer and a notes_buf in bun.default_allocator to hold the cloned message text, but Log.deinit() only does msgs.clearAndFree() and never frees those buffers — so each async transform() parse error leaks the cloned error text. This is the same pre-existing pattern as every other appendToWithRecycled caller and trading a UAF for a small error-path leak is clearly the right call here; just flagging it as a candidate for a follow-up that gives Log proper ownership of cloned string buffers.

Extended reasoning...

What the bug is

The new defer block in TransformTask.run() calls this.log.appendToWithRecycled(&new_log, true) to deep-clone any parser/lexer error messages out of the per-task arena into bun.default_allocator before the arena is freed. This correctly fixes the use-after-free. However, the buffers that appendToWithRecycled allocates to hold the cloned text are never freed, so they leak.

Code path

In src/logger.zig, cloneToWithRecycled() (called by appendToWithRecycled) does two allocations against other.msgs.allocator — which here is bun.default_allocator because new_log was created with logger.Log.init(bun.default_allocator):

  • line 818: try string_builder.allocate(other.msgs.allocator); — one contiguous buffer holding all cloned text, location.file, location.line_text, etc.
  • line 819: var notes_buf = try other.msgs.allocator.alloc(Data, notes_count); — backing storage for cloned notes.

These buffers are only referenced via slices stored inside the cloned Msg entries; Log itself keeps no separate handle to them.

TransformTask.deinit() calls this.log.deinit(), and Log.deinit() (logger.zig:840) is just log.msgs.clearAndFree(). That frees the ArrayList(Msg) backing array but does not call Msg.deinit() on the entries, and does not free the StringBuilder buffer or notes_buf. There is even an explicit TODO at logger.zig:846 noting that deinit "does not de-initialize the log".

Why nothing else frees it

After run() returns, then() runs on the JS thread and calls this.log.toJS() → Msg.clone(), which allocator.dupes the text a second time into default_allocator for the BuildMessage. So the first clone made by appendToWithRecycled is consumed only by being copied again, and is then orphaned when this.log.deinit() drops the msgs array.

Step-by-step example

  1. transpiler.transform("const x = ;;;") schedules a TransformTask.
  2. On the worker thread, run() creates arena, sets it as the transpiler allocator, parses, and the lexer pushes a Msg whose data.text = "Unexpected ;" and location.line_text = "const x = ;;;" are allocated in arena.
  3. The new defer fires: new_log = Log.init(default_allocator); appendToWithRecycled(&new_log, true) allocates a ~few-dozen-byte StringBuilder buffer in default_allocator, copies "Unexpected ;", the file path, and the line text into it, and rewrites new_log.msgs.items[0] to point at those slices. this.log = new_log.
  4. arena.deinit() runs — fine, nothing points into it anymore (UAF fixed).
  5. On the JS thread, then() → log.toJS() → Msg.clone() dupes the text again into default_allocator for the BuildMessage, then deinit() runs.
  6. this.log.deinit() → msgs.clearAndFree() frees the 1-element Msg array. The StringBuilder buffer and (empty here, but in general) notes_buf from step 3 are never freed.

Repeat 64 times (as the regression test does) → 64 small leaks.

Impact

Small and bounded: it only triggers on the error path of async transform(), and each leak is roughly the size of the error message text + line text + file path (tens of bytes). For typical usage this is negligible. It could add up for long-running tooling that repeatedly calls transform() on broken input (e.g. a watch loop over a file with a syntax error), but it will not affect correctness.

Crucially, this PR replaces a use-after-free (a real memory-safety bug producing garbage error messages / ASAN crashes) with a small error-path leak — strictly an improvement.

Why this is a nit, not a blocker

Log has no ownership model for message string content. appendToWithRecycled is the canonical pattern in this codebase for moving log messages out of a dying arena, and every existing caller (e.g. BundleThread.zig) has the identical leak. This PR is following established convention; fixing it properly requires Log to track owned string buffers (or for Log.deinit() to walk msgs and free data/notes), which is out of scope for a targeted UAF fix.

Possible follow-up fix

Either (a) have cloneToWithRecycled stash the StringBuilder.ptr[0..cap] and notes_buf on Log so deinit() can free them, or (b) make Log.deinit() iterate msgs.items and call msg.deinit(log.msgs.allocator) before clearAndFree(). (b) is risky because many call sites push arena-backed or static text into logs, so (a) — explicit ownership of the clone buffers — is probably safer.


const allocator = arena.allocator();
var ast_memory_allocator = bun.handleOom(allocator.create(JSAst.ASTMemoryAllocator));
var ast_scope = ast_memory_allocator.enter(allocator);
Expand Down
20 changes: 20 additions & 0 deletions test/js/bun/transpiler/transpiler-transform-error-uaf.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { expect, test } from "bun:test";

test("async transform() with parse errors does not read freed arena memory", async () => {
const transpiler = new Bun.Transpiler();

// Parse errors are allocated in a per-task arena that is freed when the
// worker thread finishes. Before the fix, the error text was read from
// that freed arena on the JS thread when rejecting the promise.
const results = await Promise.allSettled(
Array.from({ length: 64 }, () => transpiler.transform("const x = ;;;")),
);

for (const result of results) {
expect(result.status).toBe("rejected");
const reason = (result as PromiseRejectedResult).reason;
expect(reason.message).toBe("Unexpected ;");
expect(reason.position?.line).toBe(1);
expect(reason.position?.lineText).toBe("const x = ;;;");
}
});
Loading