mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-17 14:22:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(shell): prevent PATH_MAX panic in rm and add test

Browse Source 
Add length check before path.join in rm's root-check loop to prevent
index out of bounds panic when path exceeds the fixed-size buffer.
A path exceeding PATH_MAX can't be root, so skip the check and let
the actual operation return ENAMETOOLONG.

Also add rm test and make all PATH_MAX tests concurrent.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Dylan Conway
2026-02-13 03:07:32 +00:00
parent 68a542b4b3
commit c8b15f6d60
2 changed files with 33 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/shell/builtin/rm.zig
View File

@@ -185,6 +185,11 @@ pub noinline fn next(this: *Rm) Yield {
for (filepath_args) |filepath| {
const path = filepath[0..bun.len(filepath)];
// Skip root check if combined path exceeds buffer size —
// a path that long can't be root, and the join would panic.
// The actual rm operation will return ENAMETOOLONG.
if (!ResolvePath.Platform.auto.isAbsolute(path) and cwd.len + path.len + 1 >= bun.MAX_PATH_BYTES)
continue;
const resolved_path = if (ResolvePath.Platform.auto.isAbsolute(path)) path else bun.path.join(&[_][]const u8{ cwd, path }, .auto);
const is_root = brk: {
const normalized = bun.path.normalizeString(resolved_path, false, .auto);

31
test/js/bun/shell/shell-path-max.test.ts
View File

@@ -1,14 +1,14 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isPosix } from "harness";
// Regression test: touch and mkdir with paths exceeding PATH_MAX (4096)
// Regression test: touch, mkdir, and rm with paths exceeding PATH_MAX (4096)
// used to panic with "index out of bounds" in resolve_path.zig.
// After the fix, they return ENAMETOOLONG error instead.
describe.if(isPosix)("builtins with paths exceeding PATH_MAX should not crash", () => {
const longPath = Buffer.alloc(5000, "A").toString();
test("touch with path > PATH_MAX returns error", async () => {
test.concurrent("touch with path > PATH_MAX returns error", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
@@ -33,7 +33,7 @@ describe.if(isPosix)("builtins with paths exceeding PATH_MAX should not crash",
expect(exitCode).toBe(0);
});
test("mkdir with path > PATH_MAX returns error", async () => {
test.concurrent("mkdir with path > PATH_MAX returns error", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
@@ -56,4 +56,29 @@ describe.if(isPosix)("builtins with paths exceeding PATH_MAX should not crash",
expect(stderr).toContain("File name too long");
expect(exitCode).toBe(0);
});
test.concurrent("rm with path > PATH_MAX does not crash", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
import { $ } from "bun";
$.throws(false);
const r = await $\`rm ${longPath}\`;
console.log("exitCode:" + r.exitCode);
`,
],
env: { ...bunEnv, longPath },
stderr: "pipe",
stdout: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// rm should not crash (exit 132=illegal instruction). It should exit with an error.
expect(stdout).toContain("exitCode:");
expect(stdout).not.toContain("exitCode:0");
expect(exitCode).toBe(0);
});
});