From 4de46b9883e2766bcd78d7f5235f12c8d2bcfebf Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 30 Apr 2026 07:28:38 +0000 Subject: [PATCH 1/3] FileSystemRouter: build JS error before freeing arena in error paths The constructor's log is initialized with the arena allocator. When route loading produces errors (e.g. a filename like '[foo.tsx' missing its closing bracket), the arena was freed before log.toJS() read the messages, causing a use-after-free that ASAN catches as use-after-poison. Build the JS error value first, then free the arena. Applies to all four log.toJS() call sites in constructor() and reload(). --- src/bun.js/api/filesystem_router.zig | 16 +++++++--- test/js/bun/util/filesystem_router.test.ts | 34 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index a5da347f1cb9..8bb420365e67 100644 --- a/src/bun.js/api/filesystem_router.zig +++ b/src/bun.js/api/filesystem_router.zig @@ -109,10 +109,12 @@ pub const FileSystemRouter = struct { const path_to_use = (root_dir_path.cloneWithTrailingSlash(allocator) catch unreachable).slice(); const root_dir_info = vm.transpiler.resolver.readDirInfo(path_to_use) catch { + // Build the JS error before freeing the arena: `log` is backed by the arena allocator. + const err_value = try log.toJS(globalThis, globalThis.allocator(), "reading root directory"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "reading root directory")); + return globalThis.throwValue(err_value); } orelse { origin_str.deinit(); arena.deinit(); @@ -127,10 +129,12 @@ pub const FileSystemRouter = struct { }) catch unreachable; router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch { + // Build the JS error before freeing the arena: `log` is backed by the arena allocator. + const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes")); + return globalThis.throwValue(err_value); }; if (try argument.get(globalThis, "origin")) |origin| { @@ -143,10 +147,12 @@ pub const FileSystemRouter = struct { } if (log.errors + log.warnings > 0) { + // Build the JS error before freeing the arena: `log` is backed by the arena allocator. + const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes")); + return globalThis.throwValue(err_value); } var fs_router = globalThis.allocator().create(FileSystemRouter) catch unreachable; @@ -246,9 +252,11 @@ pub const FileSystemRouter = struct { .asset_prefix_path = this.router.config.asset_prefix_path, }) catch unreachable; router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch { + // Build the JS error before freeing the arena: `log` is backed by the arena allocator. + const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes")); + return globalThis.throwValue(err_value); }; this.router.deinit(); diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts index 02e27b5dfacd..a3338064e45e 100644 --- a/test/js/bun/util/filesystem_router.test.ts +++ b/test/js/bun/util/filesystem_router.test.ts @@ -510,3 +510,37 @@ it("MatchedRoute.params does not leak", async () => { expect(stdout).toBe(""); expect(exitCode).toBe(0); }, 60_000); + +it("throws a clean error for invalid route filenames (no use-after-free)", async () => { + // The constructor's log is backed by an arena allocator. When route loading + // produces errors (e.g. a filename like `[foo.tsx` missing its closing bracket), + // the arena must not be freed before log.toJS() reads the messages. + // Run in a subprocess so an ASAN crash doesn't take down the test runner. + using dir = tempDir("fsr-invalid-route", { + "pages/[foo.tsx": "export default 1;", + }); + + const code = /* ts */ ` + try { + new Bun.FileSystemRouter({ + style: "nextjs", + dir: ${JSON.stringify(path.join(String(dir), "pages"))}, + fileExtensions: [".tsx"], + }); + console.log("no-throw"); + } catch (e) { + console.log("caught:" + (e?.message ?? String(e))); + } + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", code], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("caught:Route is missing a closing bracket]"); + expect(exitCode).toBe(0); +}); From ed3f8bd098075b2f69643913672f599a051cf896 Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 30 Apr 2026 07:42:49 +0000 Subject: [PATCH 2/3] FileSystemRouter: run arena cleanup even if log.toJS fails Capture the error union instead of using try, so arena.deinit() and destroy(arena) run unconditionally before the result is unwrapped. --- src/bun.js/api/filesystem_router.zig | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index 8bb420365e67..9de43c45f85c 100644 --- a/src/bun.js/api/filesystem_router.zig +++ b/src/bun.js/api/filesystem_router.zig @@ -110,11 +110,12 @@ pub const FileSystemRouter = struct { const root_dir_info = vm.transpiler.resolver.readDirInfo(path_to_use) catch { // Build the JS error before freeing the arena: `log` is backed by the arena allocator. - const err_value = try log.toJS(globalThis, globalThis.allocator(), "reading root directory"); + // Capture the error union so cleanup runs even if toJS itself fails. + const err_value = log.toJS(globalThis, globalThis.allocator(), "reading root directory"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(err_value); + return globalThis.throwValue(try err_value); } orelse { origin_str.deinit(); arena.deinit(); @@ -130,11 +131,12 @@ pub const FileSystemRouter = struct { router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch { // Build the JS error before freeing the arena: `log` is backed by the arena allocator. - const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); + // Capture the error union so cleanup runs even if toJS itself fails. + const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(err_value); + return globalThis.throwValue(try err_value); }; if (try argument.get(globalThis, "origin")) |origin| { @@ -148,11 +150,12 @@ pub const FileSystemRouter = struct { if (log.errors + log.warnings > 0) { // Build the JS error before freeing the arena: `log` is backed by the arena allocator. - const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); + // Capture the error union so cleanup runs even if toJS itself fails. + const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes"); origin_str.deinit(); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(err_value); + return globalThis.throwValue(try err_value); } var fs_router = globalThis.allocator().create(FileSystemRouter) catch unreachable; @@ -253,10 +256,11 @@ pub const FileSystemRouter = struct { }) catch unreachable; router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch { // Build the JS error before freeing the arena: `log` is backed by the arena allocator. - const err_value = try log.toJS(globalThis, globalThis.allocator(), "loading routes"); + // Capture the error union so cleanup runs even if toJS itself fails. + const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes"); arena.deinit(); globalThis.allocator().destroy(arena); - return globalThis.throwValue(err_value); + return globalThis.throwValue(try err_value); }; this.router.deinit(); From 6cd9f81c3252cdbc4fe5464a6e69fce1fb4335c1 Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 30 Apr 2026 07:47:32 +0000 Subject: [PATCH 3/3] FileSystemRouter.reload: free arena on readDirInfo error Pre-existing leak: the readDirInfo catch block in reload() returned without freeing the newly-created arena. Apply the same pattern as the other four log.toJS sites. --- src/bun.js/api/filesystem_router.zig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index 9de43c45f85c..05361c3ac5e3 100644 --- a/src/bun.js/api/filesystem_router.zig +++ b/src/bun.js/api/filesystem_router.zig @@ -242,7 +242,12 @@ pub const FileSystemRouter = struct { bustDirCache(this, globalThis); const root_dir_info = vm.transpiler.resolver.readDirInfo(this.router.config.dir) catch { - return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "reading root directory")); + // Build the JS error before freeing the arena: `log` is backed by the arena allocator. + // Capture the error union so cleanup runs even if toJS itself fails. + const err_value = log.toJS(globalThis, globalThis.allocator(), "reading root directory"); + arena.deinit(); + globalThis.allocator().destroy(arena); + return globalThis.throwValue(try err_value); } orelse { arena.deinit(); globalThis.allocator().destroy(arena);