Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 22 additions & 4 deletions src/bun.js/node/path_watcher.zig
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,12 @@
const child_is_file = entry.kind != .directory;
if (dirs_only and child_is_file) continue;
const name = entry.name.slice();
// joinZBuf has no bounds check: when abs_dir is near PATH_MAX and
// name is near NAME_MAX, the normalized result overflows abs_buf.
// The resulting 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 this bound covers rel_buf too.
if (abs_dir.len + 1 + name.len + 1 > abs_buf.len) continue;
const child_abs = bun.path.joinZBuf(abs_buf, &[_][]const u8{ abs_dir, name }, .posix);
const child_rel: []const u8 = if (rel_dir.len == 0)
name
Expand Down Expand Up @@ -650,6 +656,10 @@
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.
name

Check failure on line 662 in src/bun.js/node/path_watcher.zig

View check run for this annotation

Claude / Claude Code Review

Off-by-one in rel-path bounds check: > should be >=

This guard is off by one for relative inputs: `joinStringBuf` → `normalizeStringNodeT` writes into `buf[1..]` on POSIX (resolve_path.zig:1747), so the effective output capacity is `path_buf.len - 1 = 4095`, but the check uses `> path_buf.len` and lets `subpath.len + 1 + name.len == 4096` through — overflowing `path_buf` by exactly one byte. Change `>` to `>=` (the two `joinZBuf` guards are correct as-is because their absolute first part's leading `/` cancels the reserved byte; only this relative
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
else
bun.path.joinStringBuf(&path_buf, &[_][]const u8{ owner.subpath, name }, .posix);

Expand All @@ -663,10 +673,18 @@
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);
// joinZBuf has no bounds check: when watcher.path is near
// PATH_MAX the joined absolute path overflows abs_buf.
// inotify_add_watch would return ENAMETOOLONG for such a
// path anyway, so don't attempt to register it. joinZBuf
// skips empty parts, so this upper bound is conservative
// when subpath is empty.
if (watcher.path.len + 1 + owner.subpath.len + 1 + name.len + 1 <= abs_buf.len) {
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);
}
}
}
}
Expand Down
98 changes: 97 additions & 1 deletion test/js/node/watch/fs.watch.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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);
},
);
Loading