diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 72bac26a971..00ee65b3e2a 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -4673,7 +4673,7 @@ pub const NodeFS = struct { const path_parts = [_]string{ root_basename, basename }; return .{ - .err = err.withPath(bun.path.joinZBuf(buf, &path_parts, .auto)), + .err = err.withPath(bun.path.joinZBufWithoutBoundsCheck(buf, &path_parts, .auto)), }; } return .{ @@ -4705,7 +4705,7 @@ pub const NodeFS = struct { if (comptime !is_root) { const path_parts = [_]string{ root_basename, basename }; return .{ - .err = err.withPath(bun.path.joinZBuf(buf, &path_parts, .auto)), + .err = err.withPath(bun.path.joinZBufWithoutBoundsCheck(buf, &path_parts, .auto)), }; } @@ -4723,7 +4723,7 @@ pub const NodeFS = struct { } const path_parts = [_]string{ basename, utf8_name }; - break :brk bun.path.joinZBuf(buf, &path_parts, .auto); + break :brk bun.path.joinZBufWithoutBoundsCheck(buf, &path_parts, .auto); }; // Track effective kind - may be resolved from .unknown via stat @@ -4891,7 +4891,7 @@ pub const NodeFS = struct { } const path_parts = [_]string{ basename, utf8_name }; - break :brk bun.path.joinZBuf(buf, &path_parts, .auto); + break :brk bun.path.joinZBufWithoutBoundsCheck(buf, &path_parts, .auto); }; // Track effective kind - may be resolved from .unknown via stat diff --git a/src/bun.js/node/path_watcher.zig b/src/bun.js/node/path_watcher.zig index 4e584838a24..ebca5f7b027 100644 --- a/src/bun.js/node/path_watcher.zig +++ b/src/bun.js/node/path_watcher.zig @@ -384,7 +384,11 @@ fn walkSubtree( const child_is_file = entry.kind != .directory; if (dirs_only and child_is_file) continue; const name = entry.name.slice(); - const child_abs = bun.path.joinZBuf(abs_buf, &[_][]const u8{ abs_dir, name }, .posix); + // When abs_dir is near PATH_MAX and name is near NAME_MAX, the joined + // absolute path is unrepresentable (inotify_add_watch / open would + // return ENAMETOOLONG), so skip the entry. rel_dir is a strict suffix + // of abs_dir, so the rel_buf join below is covered by the same bound. + const child_abs = bun.path.joinZBuf(abs_buf, &[_][]const u8{ abs_dir, name }, .posix) catch continue; const child_rel: []const u8 = if (rel_dir.len == 0) name else @@ -650,6 +654,13 @@ const Linux = struct { name else if (name.len == 0) owner.subpath + else if (owner.subpath.len + 1 + name.len >= path_buf.len) + // joinStringBuf has no bounds check; fall back to the bare + // filename rather than overflowing path_buf. `>=` not `>`: + // on POSIX normalizeStringNodeT reserves buf[0] for a + // possible leading '/', so relative output capacity is + // path_buf.len - 1. + name else bun.path.joinStringBuf(&path_buf, &[_][]const u8{ owner.subpath, name }, .posix); @@ -663,10 +674,14 @@ const Linux = struct { if (watcher.recursive and is_dir_child and (ev.mask & (IN.CREATE | IN.MOVED_TO) != 0) and name.len > 0) { const abs_buf = bun.path_buffer_pool.get(); defer bun.path_buffer_pool.put(abs_buf); - const child_abs = bun.path.joinZBuf(abs_buf, &[_][]const u8{ watcher.path, owner.subpath, name }, .posix); - // These may rehash `wd_map`; `owners` is re-fetched next iteration. - _ = addOne(manager, watcher, child_abs, rel); - walkAndAdd(manager, watcher, child_abs, rel); + // When watcher.path is near PATH_MAX the joined absolute + // path is unrepresentable; inotify_add_watch would return + // ENAMETOOLONG anyway, so skip registration. + if (bun.path.joinZBuf(abs_buf, &[_][]const u8{ watcher.path, owner.subpath, name }, .posix)) |child_abs| { + // These may rehash `wd_map`; `owners` is re-fetched next iteration. + _ = addOne(manager, watcher, child_abs, rel); + walkAndAdd(manager, watcher, child_abs, rel); + } else |_| {} } } } diff --git a/src/glob/GlobWalker.zig b/src/glob/GlobWalker.zig index a80d38a28ab..61bd4add3f9 100644 --- a/src/glob/GlobWalker.zig +++ b/src/glob/GlobWalker.zig @@ -106,7 +106,7 @@ pub fn statatWindows(fd: bun.FD, path: [:0]const u8) Maybe(bun.Stat) { dir[0..dir.len], path, }; - const statpath = ResolvePath.joinZBuf(&buf, parts, .auto); + const statpath = ResolvePath.joinZBufWithoutBoundsCheck(&buf, parts, .auto); return Syscall.stat(statpath); } diff --git a/src/install/PackageInstaller.zig b/src/install/PackageInstaller.zig index 60a9fee746c..08598cb34c8 100644 --- a/src/install/PackageInstaller.zig +++ b/src/install/PackageInstaller.zig @@ -58,7 +58,7 @@ pub const PackageInstaller = struct { noinline fn directoryExistsAtWithoutOpeningDirectories(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, file_path: [:0]const u8) bool { var path_buf: bun.PathBuffer = undefined; const parts: [2][]const u8 = .{ this.path.items, file_path }; - return bun.sys.directoryExistsAt(.fromStdDir(root_node_modules_dir), bun.path.joinZBuf(&path_buf, &parts, .auto)).unwrapOr(false); + return bun.sys.directoryExistsAt(.fromStdDir(root_node_modules_dir), bun.path.joinZBufWithoutBoundsCheck(&path_buf, &parts, .auto)).unwrapOr(false); } pub fn directoryExistsAt(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, file_path: [:0]const u8) bool { @@ -75,7 +75,7 @@ pub const PackageInstaller = struct { noinline fn openFileWithoutOpeningDirectories(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, file_path: [:0]const u8) bun.sys.Maybe(bun.sys.File) { var path_buf: bun.PathBuffer = undefined; const parts: [2][]const u8 = .{ this.path.items, file_path }; - return bun.sys.File.openat(.fromStdDir(root_node_modules_dir), bun.path.joinZBuf(&path_buf, &parts, .auto), bun.O.RDONLY, 0); + return bun.sys.File.openat(.fromStdDir(root_node_modules_dir), bun.path.joinZBufWithoutBoundsCheck(&path_buf, &parts, .auto), bun.O.RDONLY, 0); } pub fn readFile(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, file_path: [:0]const u8, allocator: std.mem.Allocator) !bun.sys.File.ReadToEndResult { diff --git a/src/install/PackageManager/patchPackage.zig b/src/install/PackageManager/patchPackage.zig index 032ae68d1ab..53102a4a20e 100644 --- a/src/install/PackageManager/patchPackage.zig +++ b/src/install/PackageManager/patchPackage.zig @@ -162,7 +162,7 @@ pub fn doPatchCommit( const name, const version = Dependency.splitNameAndMaybeVersion(argument); const pkg_id, const node_modules = pkgInfoForNameAndVersion(lockfile, &iterator, argument, name, version); - const changes_dir = bun.path.joinZBuf(pathbuf[0..], &[_][]const u8{ + const changes_dir = bun.path.joinZBufWithoutBoundsCheck(pathbuf[0..], &[_][]const u8{ node_modules.relative_path, name, }, .auto); diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig index 119a2758b1a..f9b2b990803 100644 --- a/src/install/extract_tarball.zig +++ b/src/install/extract_tarball.zig @@ -505,7 +505,7 @@ pub fn moveToCacheDirectory( }) { const json_file, json_buf = bun.sys.File.readFileFrom( bun.FD.fromStdDir(cache_dir), - bun.path.joinZBuf(&json_path_buf, &[_]string{ folder_name, "package.json" }, .auto), + bun.path.joinZBufWithoutBoundsCheck(&json_path_buf, &[_]string{ folder_name, "package.json" }, .auto), bun.default_allocator, ).unwrap() catch |err| { if (this.resolution.tag == .github and err == error.ENOENT) { diff --git a/src/install/patch_install.zig b/src/install/patch_install.zig index 385cd77dd8e..f2eda0ef3df 100644 --- a/src/install/patch_install.zig +++ b/src/install/patch_install.zig @@ -236,7 +236,7 @@ pub const PatchTask = struct { var absolute_patchfile_path_buf: bun.PathBuffer = undefined; // 1. Parse the patch file - const absolute_patchfile_path = bun.path.joinZBuf(&absolute_patchfile_path_buf, &[_][]const u8{ + const absolute_patchfile_path = bun.path.joinZBufWithoutBoundsCheck(&absolute_patchfile_path_buf, &[_][]const u8{ dir, patchfile_path, }, .auto); @@ -375,7 +375,7 @@ pub const PatchTask = struct { // 6. rename to cache dir var path_in_tmpdir_buf: bun.PathBuffer = undefined; - const path_in_tmpdir = bun.path.joinZBuf( + const path_in_tmpdir = bun.path.joinZBufWithoutBoundsCheck( &path_in_tmpdir_buf, &[_][]const u8{ tempdir_name, @@ -407,7 +407,7 @@ pub const PatchTask = struct { var absolute_patchfile_path_buf: bun.PathBuffer = undefined; // parse the patch file - const absolute_patchfile_path = bun.path.joinZBuf( + const absolute_patchfile_path = bun.path.joinZBufWithoutBoundsCheck( &absolute_patchfile_path_buf, &[_][]const u8{ dir, diff --git a/src/patch.zig b/src/patch.zig index 0eee72d1cf3..58f51735d63 100644 --- a/src/patch.zig +++ b/src/patch.zig @@ -179,7 +179,7 @@ pub const PatchFile = struct { .err => |e| return e.withoutPath(), }; var buf: bun.PathBuffer = undefined; - const joined_absfilepath = bun.path.joinZBuf(&buf, &[_][]const u8{ absfilepath, filepath }, .auto); + const joined_absfilepath = bun.path.joinZBufWithoutBoundsCheck(&buf, &[_][]const u8{ absfilepath, filepath }, .auto); const fd = switch (bun.sys.open(joined_absfilepath, bun.O.RDWR, 0)) { .err => |e| return e.withoutPath(), .result => |f| f, diff --git a/src/repl.zig b/src/repl.zig index a4cfbe96ccf..25c08c11653 100644 --- a/src/repl.zig +++ b/src/repl.zig @@ -186,7 +186,7 @@ const History = struct { if (home_path.len == 0) return; var path_buf: bun.PathBuffer = undefined; - const path = bun.path.joinZBuf(&path_buf, &[_][]const u8{ home_path, HISTORY_FILENAME }, .auto); + const path = bun.path.joinZBufWithoutBoundsCheck(&path_buf, &[_][]const u8{ home_path, HISTORY_FILENAME }, .auto); self.file_path = try self.allocator.dupe(u8, path); const content = switch (bun.sys.File.readFrom(bun.FD.cwd(), path, self.allocator)) { diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig index 04d8401b541..a2e69b0e6ce 100644 --- a/src/resolver/resolve_path.zig +++ b/src/resolver/resolve_path.zig @@ -1219,16 +1219,53 @@ pub fn join(_parts: anytype, comptime platform: Platform) []const u8 { return joinStringBuf(&join_buf, _parts, platform); } pub fn joinZ(_parts: anytype, comptime platform: Platform) [:0]const u8 { - return joinZBuf(&join_buf, _parts, platform); + return joinZBufWithoutBoundsCheck(&join_buf, _parts, platform); } -pub fn joinZBuf(buf: []u8, _parts: anytype, comptime platform: Platform) [:0]const u8 { +/// Join `_parts` with separators and normalize into `buf`, nul-terminated. +/// +/// Returns `error.NameTooLong` when the concatenated input length exceeds +/// what `buf` can hold. The underlying `joinStringBuf` -> `normalizeStringNodeT` +/// pipeline has no output bounds check (`normalizeStringGenericTZ` writes +/// segments via unchecked `@memcpy`), so without this guard a too-long input +/// overflows `buf` — a safety panic in debug, UB in ReleaseFast. The check is +/// against the pre-normalization concatenated length; normalization can only +/// shrink, so a result that would fit after collapsing `..`/`.`/`//` may be +/// conservatively rejected. +/// +/// Callers that have already ensured the result fits (or want the legacy +/// panic-on-overflow) should use `joinZBufWithoutBoundsCheck`. +pub fn joinZBuf(buf: []u8, _parts: anytype, comptime platform: Platform) error{NameTooLong}![:0]const u8 { + var upper: usize = 0; + var first: u8 = 0; + for (_parts) |p| { + if (p.len == 0) continue; + if (upper == 0) first = p[0]; + if (upper > 0) upper += 1; // separator between non-empty parts + upper += p.len; + } + // One byte is reserved for the trailing nul. On non-Windows platforms + // `normalizeStringNodeT` additionally writes the body into `buf[1..]`, + // reserving `buf[0]` for a leading separator on absolute results; for + // relative inputs that byte is dead, so effective capacity is one less. + const leading_sep = first != 0 and platform.isSeparator(first); + const reserve: usize = if (platform == .windows or leading_sep) 1 else 2; + if (upper + reserve > buf.len) return error.NameTooLong; + const joined = joinStringBuf(buf[0 .. buf.len - 1], _parts, platform); assert(bun.isSliceInBuffer(joined, buf)); const start_offset = @intFromPtr(joined.ptr) - @intFromPtr(buf.ptr); buf[joined.len + start_offset] = 0; return buf[start_offset..][0..joined.len :0]; } + +/// Like `joinZBuf` but panics instead of returning an error when the result +/// would overflow `buf`. Use this when the inputs are known to fit (e.g. a +/// bounded number of fixed-length segments) and the error path would be +/// unreachable anyway. +pub fn joinZBufWithoutBoundsCheck(buf: []u8, _parts: anytype, comptime platform: Platform) [:0]const u8 { + return joinZBuf(buf, _parts, platform) catch @panic("joinZBuf: out of bounds"); +} pub fn joinStringBuf(buf: []u8, parts: anytype, comptime platform: Platform) []const u8 { return joinStringBufT(u8, buf, parts, platform); } diff --git a/src/shell/builtin/cp.zig b/src/shell/builtin/cp.zig index acbe72ff65b..dbb67889c7a 100644 --- a/src/shell/builtin/cp.zig +++ b/src/shell/builtin/cp.zig @@ -456,7 +456,7 @@ pub const ShellCpTask = struct { this.cwd_path[0..], this.tgt[0..], }; - break :brk ResolvePath.joinZBuf(buf2[0..bun.MAX_PATH_BYTES], parts, .auto); + break :brk ResolvePath.joinZBufWithoutBoundsCheck(buf2[0..bun.MAX_PATH_BYTES], parts, .auto); }; // Cases: @@ -514,7 +514,7 @@ pub const ShellCpTask = struct { tgt[0..tgt.len], basename, }; - tgt = ResolvePath.joinZBuf(buf3[0..bun.MAX_PATH_BYTES], parts, .auto); + tgt = ResolvePath.joinZBufWithoutBoundsCheck(buf3[0..bun.MAX_PATH_BYTES], parts, .auto); } else if (this.operands == 2) { // source_dir -> new_target_dir } else { @@ -532,7 +532,7 @@ pub const ShellCpTask = struct { tgt[0..tgt.len], basename, }; - tgt = ResolvePath.joinZBuf(buf3[0..bun.MAX_PATH_BYTES], parts, .auto); + tgt = ResolvePath.joinZBufWithoutBoundsCheck(buf3[0..bun.MAX_PATH_BYTES], parts, .auto); copying_many = true; } diff --git a/src/shell/builtin/rm.zig b/src/shell/builtin/rm.zig index d249d9f52a6..e346a939a45 100644 --- a/src/shell/builtin/rm.zig +++ b/src/shell/builtin/rm.zig @@ -809,8 +809,8 @@ pub const ShellRmTask = struct { pub fn bufJoin(this: *ShellRmTask, buf: *bun.PathBuffer, parts: []const []const u8, _: Syscall.Tag) Maybe([:0]const u8) { if (this.join_style == .posix) { - return .{ .result = ResolvePath.joinZBuf(buf, parts, .posix) }; - } else return .{ .result = ResolvePath.joinZBuf(buf, parts, .windows) }; + return .{ .result = ResolvePath.joinZBufWithoutBoundsCheck(buf, parts, .posix) }; + } else return .{ .result = ResolvePath.joinZBufWithoutBoundsCheck(buf, parts, .windows) }; } pub fn removeEntry(this: *ShellRmTask, dir_task: *DirTask, is_absolute: bool) Maybe(void) { diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 32777d42c98..e1d1b485f12 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -1840,7 +1840,7 @@ pub const ShellSyscall = struct { dirpath[0..dirpath.len], to[0..to.len], }; - const joined = ResolvePath.joinZBuf(buf, parts, .auto); + const joined = ResolvePath.joinZBufWithoutBoundsCheck(buf, parts, .auto); return .{ .result = joined }; } diff --git a/test/js/node/watch/fs.watch.test.ts b/test/js/node/watch/fs.watch.test.ts index 2c38aabf319..19ec56ef1b4 100644 --- a/test/js/node/watch/fs.watch.test.ts +++ b/test/js/node/watch/fs.watch.test.ts @@ -1,5 +1,15 @@ import { pathToFileURL } from "bun"; -import { bunEnv, bunExe, bunRun, bunRunAsScript, isMacOS, isWindows, tempDir, tempDirWithFiles } from "harness"; +import { + bunEnv, + bunExe, + bunRun, + bunRunAsScript, + isLinux, + isMacOS, + isWindows, + tempDir, + tempDirWithFiles, +} from "harness"; import fs, { FSWatcher } from "node:fs"; import path from "path"; @@ -900,3 +910,89 @@ test.skipIf(!isMacOS)("fs.watch(dir) on macOS does not leak the resolved FSEvent expect(exitCode).toBe(0); expect(stdout).toContain("RSS growth:"); }); + +// The Linux backend joins the watched directory's absolute path with child +// names into a bun.PathBuffer (4096 bytes) via joinZBuf/joinStringBuf with no +// bounds check. A watched directory whose absolute path is near PATH_MAX plus +// a NAME_MAX (255) entry overflows the buffer — a safety panic in debug/ASAN, +// silent corruption in release. Linux-only: macOS uses FSEvents and Windows +// uses win_watcher.zig. Exercises four code paths: +// - non-recursive watch + create long-named file (inotify dispatch) +// - recursive watch + pre-existing long-named subdir (walkSubtree at +// registration time) +// - recursive watch + create long-named subdir (new-directory handling in +// the inotify reader thread, which rebuilds the absolute path to register +// a watch on it) +// - recursive watch + create long-named file +test.skipIf(!isLinux)( + "fs.watch on a near-PATH_MAX directory does not overflow when a long-named entry is created inside", + async () => { + using dir = tempDir("fs-watch-pathmax-overflow", {}); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* ts */ ` + const fs = require("fs"); + const path = require("path"); + const base = process.argv[1]; + + // Build a directory tree whose absolute path exceeds 3840 bytes so + // abs + sep + 255-byte name is guaranteed > 4096 regardless of the + // TMPDIR base length. Create each segment via a relative mkdir so + // the per-call path stays well under PATH_MAX. + const seg = Buffer.alloc(200, "d").toString(); + process.chdir(base); + let abs = base; + let rel = "."; + while (abs.length + 1 + seg.length < 4050) { + rel = path.join(rel, seg); + abs = path.join(abs, seg); + fs.mkdirSync(rel); + } + // abs.length is now in [3849, 4049]; abs + "/" + 255-byte name > 4096. + process.chdir(rel); + + // Pre-existing NAME_MAX-length subdirectory so the recursive watch's + // initial walkSubtree sees an entry whose absolute path won't fit. + const longSub = Buffer.alloc(255, "s").toString(); + fs.mkdirSync(longSub); + + const wN = fs.watch(abs, () => {}); + const wR = fs.watch(abs, { recursive: true }, () => {}); + for (const w of [wN, wR]) w.on("error", () => {}); + + // Create a NAME_MAX-length file and a second NAME_MAX-length + // subdirectory inside the watched directory. Relative paths (cwd = + // deep dir) keep each syscall under PATH_MAX. The subdirectory + // IN_CREATE|IN_ISDIR event makes the recursive watcher rebuild the + // absolute child path to register a new inotify watch on it. + const longFile = Buffer.alloc(255, "f").toString(); + const longSub2 = Buffer.alloc(254, "S").toString() + "2"; + let i = 0; + const timer = setInterval(() => { + fs.writeFileSync(longFile, "x"); + try { fs.mkdirSync(longSub2); } catch {} + if (++i > 10) { + clearInterval(timer); + wN.close(); + wR.close(); + console.log("OK " + abs.length); + } + }, 20); + `, + String(dir), + ], + 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).toStartWith("OK "); + expect(exitCode).toBe(0); + }, +);