Skip to content

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

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 4 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4906852
Fix use-after-free in Bun.Transpiler async transform() errors
robobun May 5, 2026
0a32150
re-trigger gate
robobun May 5, 2026
0ec4702
Use bun.handleOom and free cloned log messages in deinit
robobun May 5, 2026
e2618c9
retry gate
robobun May 5, 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
3 changes: 3 additions & 0 deletions src/runtime/api/JSTranspiler.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,9 @@ pub const TransformTask = struct {
this.transpiler.setAllocator(allocator);
this.transpiler.setLog(&this.log);
this.log.msgs.allocator = bun.default_allocator;
defer for (this.log.msgs.items) |*msg| {
msg.* = msg.clone(bun.default_allocator) catch msg.*;
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

@dylan-conway dylan-conway May 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@robobun fix this to properly handle OOM

@robobun robobun May 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in 0ec4702 — now uses bun.handleOom(msg.clone(bun.default_allocator)).

};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +506 to +508

@claude claude Bot May 5, 2026 •
edited by robobun
Loading

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.

Done — added msg.deinit(bun.default_allocator) for each message in TransformTask.deinit() before log.deinit().

Comment on lines +506 to +508

@claude claude Bot May 5, 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: the msg.deinit(bun.default_allocator) cleanup added in TransformTask.deinit() won't actually free everything msg.clone() allocated — Location.deinit is a no-op (logger.zig:149), so the duped location.file and location.line_text strings (for the message and each note) leak on the global heap per failing async transform(). Tiny error-path-only leak and the root asymmetry is pre-existing in Location, so not a blocker for this UAF fix — but if you want full cleanup here you'd need to free those two fields manually (or fix Location.deinit).

Extended reasoning...

What the bug is

The new defer in TransformTask.run() deep-clones each log message into bun.default_allocator:

defer for (this.log.msgs.items) |*msg| {
    msg.* = bun.handleOom(msg.clone(bun.default_allocator));
};

Msg.clone → Data.clone → Location.clone (src/logger/logger.zig:113-123), which does allocator.dupe(u8, this.file) and allocator.dupe(u8, this.line_text.?). And Msg.clone also calls bun.clone(this.notes, allocator), which (per src/bun.zig:551-557) detects Data.clone and deep-clones every note, so each note's location.file / location.line_text is also duped onto bun.default_allocator.

The matching cleanup added in TransformTask.deinit():

for (this.log.msgs.items) |*msg| msg.deinit(bun.default_allocator);

calls Msg.deinit → Data.deinit → Location.deinit. But Location.deinit is intentionally stubbed:

// don't really know what's safe to deinit here!
pub fn deinit(_: *Location, _: std.mem.Allocator) void {}

So Data.deinit frees .text and the notes slice is freed, but every Location.file and Location.line_text that was just duped onto the global heap is never freed.

Code path that triggers it

  1. transpiler.transform("const {a, a} = b") schedules a TransformTask.
  2. run() parses on a worker thread; the parser pushes a Msg with data.location = {file: "input.tsx", line_text: "const {a, a} = b"} and one note with its own location, all allocated from the per-task MimallocArena.
  3. On scope exit, the new defer runs first (LIFO) and msg.clone(bun.default_allocator) heap-dupes: data.text, data.location.file, data.location.line_text, the notes slice, notes[0].text, notes[0].location.file, notes[0].location.line_text.
  4. arena.deinit() reclaims the originals (correct — that's the UAF fix).
  5. then() runs on the main thread, builds the JS error, then calls this.deinit().
  6. deinit() runs msg.deinit(bun.default_allocator) for each message → frees data.text, notes[0].text, and the notes slice. Location.deinit is a no-op, so the four duped location strings (2 × file + 2 × line_text) stay on the global mimalloc heap forever.

Why existing code doesn't prevent it

  • this.log.deinit() only frees the msgs list backing storage, not per-message payloads.
  • Msg.deinit is the maximally-correct API to call here and the author called it; the gap is one layer down in Location.deinit, which has carried the comment "don't really know what's safe to deinit here!" since long before this PR.
  • Before this PR, these strings lived in the per-task arena and were swept by arena.deinit() (with a UAF, which this PR correctly fixes). Moving them to bun.default_allocator is what makes the no-op Location.deinit matter.

Impact

Per failing async transform(): (1 + #notes) × (len(file) + len(line_text)) bytes leaked from the global heap. For the PR's own test ("const {a, a} = b", 1 note), that's ≈ 2 × ("input.tsx" + "const {a, a} = b") ≈ 50 bytes per call. Error-path only; the success path has no log messages and leaks nothing.

Worth noting: the same Location.clone/Location.deinit asymmetry already bites BuildMessage.create → BuildMessage.finalize on this exact path (BuildMessage.zig:54/186), so this PR roughly doubles a pre-existing per-error leak rather than introducing a brand-new leak class.

Step-by-step proof

For one call to transpiler.transform("const {a, a} = b"):

Allocation in msg.clone(bun.default_allocator) Freed by msg.deinit(bun.default_allocator)?
data.text ('"a" has already been declared') ✅ Data.deinit line 214
data.location.file ("input.tsx") ❌ Location.deinit no-op line 149
data.location.line_text ("const {a, a} = b") ❌ Location.deinit no-op line 149
notes slice ([1]Data) ✅ Msg.deinit line 484
notes[0].text ✅ Data.deinit line 214
notes[0].location.file ❌ Location.deinit no-op
notes[0].location.line_text ❌ Location.deinit no-op

Four small allocations leak per failed transform.

How to fix

Either locally in TransformTask.deinit():

for (this.log.msgs.items) |*msg| {
    if (msg.data.location) |*loc| {
        bun.default_allocator.free(loc.file);
        if (loc.line_text) |lt| bun.default_allocator.free(lt);
    }
    for (msg.notes) |*note| if (note.location) |*loc| {
        bun.default_allocator.free(loc.file);
        if (loc.line_text) |lt| bun.default_allocator.free(lt);
    };
    msg.deinit(bun.default_allocator);
}

…or, better long-term, make Location.deinit actually free what Location.clone allocates (which would also fix the identical leak in BuildMessage.finalize). The latter is out of scope for this PR; flagging as a nit since the author explicitly added msg.deinit intending full cleanup.


const jsx = if (this.tsconfig != null)
this.tsconfig.?.mergeJSX(this.transpiler.options.jsx)
Expand Down
44 changes: 44 additions & 0 deletions test/js/bun/transpiler/transpiler-async-error-uaf.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { expect, test } from "bun:test";

test("concurrent async transform() rejections do not use-after-free", async () => {
const transpiler = new Bun.Transpiler({ loader: "tsx" });

const inputs = [
"const {a, a} = b",
"class X { @invalid }",
"const x: string = ;",
"@#$%^ invalid syntax !!!",
"function (",
];

const promises: Promise<unknown>[] = [];
for (let i = 0; i < 20; i++) {
for (const input of inputs) {
promises.push(transpiler.transform(input).catch(e => e));
}
}

const results = await Promise.all(promises);
expect(results).toHaveLength(20 * inputs.length);
for (const result of results) {
expect(result).toBeDefined();
expect(typeof result).toBe("object");
}
});

test("async transform() error preserves message and notes", async () => {
const transpiler = new Bun.Transpiler({ loader: "tsx" });

const errors = await Promise.all(
Array.from({ length: 8 }, () => transpiler.transform("const {a, a} = b").catch(e => e)),
);

for (const err of errors) {
expect(err.message).toBe('"a" has already been declared');
expect(err.position?.file).toBe("input.tsx");
expect(err.position?.lineText).toBe("const {a, a} = b");
expect(err.notes).toHaveLength(1);
expect(err.notes[0].message).toBe('"a" was originally declared here');
expect(err.notes[0].position?.lineText).toBe("const {a, a} = b");
}
});
Loading