Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1749,6 +1749,7 @@ pub(crate) fn mmap_file(global_this: &JSGlobalObject, callframe: &CallFrame) ->
global_this.throw_invalid_arguments(format_args!("Path too long"))
);
}
node::Valid::no_null_bytes(path_str.slice(), "path", "a string", global_this)?;
let paths = &[path_str.slice()];
break 'brk bun_paths::resolve_path::join_abs_string_buf::<
bun_paths::resolve_path::platform::Auto,
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/api/filesystem_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use bun_router::{self as Router, Match as RouterMatch, RouteConfig};
use bun_url::{CombinedScanner, QueryStringMap, URL, route_param};

use crate::api::bun_object;
use crate::node::Valid;
use crate::webcore::{Request, Response};
use bun_bundler as Transpiler;

Expand Down Expand Up @@ -157,6 +158,7 @@ impl FileSystemRouter {
.throw_invalid_arguments(format_args!("Expected dir to be a string")));
}
let root_dir_path_ = dir.to_slice(global_this)?;
Valid::no_null_bytes(root_dir_path_.slice(), "dir", "a string", global_this)?;
if !(root_dir_path_.slice().is_empty() || root_dir_path_.slice() == b".") {
// resolve relative path if needed
let path_ = root_dir_path_.slice();
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/api/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use bun_paths::resolve_path::join_string_buf;
use bun_paths::{self as resolve_path, MAX_PATH_BYTES, PathBuffer, platform};
use bun_sys as syscall;

use crate::node::Valid;

// Codegen hooks (JSGlob): toJS / fromJS / fromJSDirect are provided by the
// generated C++ wrapper. See PORTING.md §JSC ".classes.ts-backed types".
#[bun_jsc::JsClass]
Expand Down Expand Up @@ -52,6 +54,7 @@ impl ScanOpts {
fn_name, MAX_PATH_BYTES
)));
}
Valid::no_null_bytes(cwd_utf8.slice(), "cwd", "a string", global_this)?;

// If its absolute return as is
if resolve_path::Platform::AUTO.is_absolute(cwd_utf8.slice()) {
Expand Down Expand Up @@ -301,6 +304,9 @@ impl Glob {
fn_name: &'static str,
arena: &mut Arena,
) -> JsResult<Option<Box<GlobWalker>>> {
// The pattern's literal prefix is opened as a C path; `match()` never
// touches the filesystem, so only scan/scanSync reject interior NULs.
Valid::no_null_bytes(&self.pattern, "pattern", "a string", global_this)?;
let Some(match_opts) = ScanOpts::from_js(global_this, arguments, fn_name, arena)? else {
return Ok(None);
};
Expand Down
34 changes: 23 additions & 11 deletions src/runtime/node/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1436,18 +1436,30 @@ impl Valid {
}

pub fn path_null_bytes(slice: &[u8], global: &JSGlobalObject) -> JsResult<()> {
if strings::index_of_char(slice, 0).is_some() {
return Err(global
.err(
jsc::ErrorCode::INVALID_ARG_VALUE,
format_args!(
"The argument 'path' must be a string, Uint8Array, or URL without null bytes. Received {}",
bun_fmt::quote(slice)
),
)
.throw());
Self::no_null_bytes(slice, "path", "a string, Uint8Array, or URL", global)
}

/// Reject interior NUL bytes in a path-like string argument with Node's
/// `ERR_INVALID_ARG_VALUE`. C paths stop at the first NUL, so a value
/// that passed JS-level validation would reach the syscall truncated.
pub fn no_null_bytes(
slice: &[u8],
name: &str,
accepts: &str,
global: &JSGlobalObject,
) -> JsResult<()> {
if strings::index_of_char(slice, 0).is_none() {
return Ok(());
}
Ok(())
Err(global
.err(
jsc::ErrorCode::INVALID_ARG_VALUE,
format_args!(
"The argument '{name}' must be {accepts} without null bytes. Received {}",
bun_fmt::quote(slice)
),
)
.throw())
}
}

Expand Down
46 changes: 46 additions & 0 deletions test/js/bun/glob/scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1125,3 +1125,49 @@ describe.skipIf(!canCreateDirSymlink)("literal path segment through a symlinked
expect(norm(result)).toEqual(["linkdir/file.txt"]);
});
});

// The cwd and the pattern's literal prefix both reach the OS as C strings,
// which stop at the first NUL: "<dir>/safe\0/../secret" would scan "<dir>/safe"
// while JS-level validation saw a path resolving to "<dir>/secret".
describe("glob scan rejects interior null bytes", () => {
const nulError = expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" });

function makeNulTree(prefix: string) {
return tempDir(prefix, {
"safe/inside.ts": "x",
"secret/leaked.ts": "x",
});
}

test("scanSync rejects a cwd option containing a null byte", () => {
using dir = makeNulTree("glob-nul-cwd-sync");
const evil = `${dir}/safe\0/../secret`;
expect(() => Array.from(new Glob("*.ts").scanSync({ cwd: evil }))).toThrow(nulError);
expect(() => Array.from(new Glob("*.ts").scanSync(evil))).toThrow(nulError);
});

// `scan` runs the same argument validation as `scanSync` before scheduling
// any work, so a bad cwd/pattern throws from the `scan()` call itself.
test("scan rejects a cwd option containing a null byte", () => {
using dir = makeNulTree("glob-nul-cwd-async");
const evil = `${dir}/safe\0/../secret`;
expect(() => new Glob("*.ts").scan({ cwd: evil })).toThrow(nulError);
expect(() => new Glob("*.ts").scan(evil)).toThrow(nulError);
});

test("scanSync rejects a pattern containing a null byte", () => {
using dir = makeNulTree("glob-nul-pattern-sync");
expect(() => Array.from(new Glob(`${dir}/secret\0trailer/*.ts`).scanSync())).toThrow(nulError);
});

test("scan rejects a pattern containing a null byte", () => {
using dir = makeNulTree("glob-nul-pattern-async");
expect(() => new Glob(`${dir}/secret\0trailer/*.ts`).scan()).toThrow(nulError);
});

test("match never touches the filesystem so null bytes stay plain characters", () => {
expect(new Glob("a\0b").match("a\0b")).toBe(true);
expect(new Glob("a\0b").match("ab")).toBe(false);
expect(new Glob("*.ts").match("a\0b.ts")).toBe(true);
});
});
36 changes: 20 additions & 16 deletions test/js/bun/util/filesystem_router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,17 @@ it("dir should be validated", async () => {
}).toThrow("Expected dir to be a string");
});

it("dir with an interior null byte is rejected", () => {
const { dir } = make(["index.tsx"]);
// The C path stops at the first NUL, so "<dir>\0suffix" would otherwise
// list <dir> while route resolution used a different JS-level path.
const nulError = expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" });
expect(() => new FileSystemRouter({ style: "nextjs", dir: `${dir}\0suffix` })).toThrow(nulError);
expect(() => new FileSystemRouter({ style: "nextjs", dir: `${dir}\0suffix` })).toThrow(
"must be a string without null bytes",
);
});

it("origin should be validated", async () => {
const { dir } = make(["posts.tsx"]);

Expand Down Expand Up @@ -682,21 +693,14 @@ it("caps the number of parsed query string parameters instead of crashing", asyn
});

it("does not match a dynamic route whose static segment merely collides on length and 32-bit hash", () => {
const low32 = (input: string) => Number(BigInt.asUintN(32, BigInt(Bun.hash.wyhash(input))));
const seen = new Map<number, string>();
let pair: [string, string] | null = null;
for (let i = 0; i < 600_000; i++) {
const candidate = "s" + i.toString(36).padStart(9, "0");
const h = low32(candidate);
const prev = seen.get(h);
if (prev !== undefined) {
pair = [prev, candidate];
break;
}
seen.set(h, candidate);
}
expect(pair).not.toBeNull();
const [routeSegment, collidingSegment] = pair!;
// Route segment matching must compare bytes, not just (length, truncated
// 32-bit wyhash). Bun.hash.wyhash(s, 0) is the same hash the router stores
// for static route segments. This equal-length pair was found by a birthday
// search; the hash32 assertion keeps the test honest if wyhash ever changes.
const routeSegment = "s000000io9";
const collidingSegment = "s000001eqf";
const hash32 = (s: string) => Number(BigInt.asUintN(32, BigInt(Bun.hash.wyhash(s))));
expect(hash32(collidingSegment)).toBe(hash32(routeSegment));
expect(collidingSegment).not.toBe(routeSegment);
expect(collidingSegment.length).toBe(routeSegment.length);

Expand All @@ -708,7 +712,7 @@ it("does not match a dynamic route whose static segment merely collides on lengt

expect(router.match(`/${routeSegment}/42`)?.name).toBe(`/${routeSegment}/[id]`);
expect(router.match(`/${collidingSegment}/42`)).toBeNull();
}, 60_000);
});

it("match() does not panic on a leading '?' or a path that percent-decodes to empty", async () => {
// URLPath::parse assumed the decoded pathname was non-empty and had a leading
Expand Down
7 changes: 7 additions & 0 deletions test/js/bun/util/mmap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ describe.skipIf(isWindows)("Bun.mmap", async () => {
expect(() => Bun.mmap(path, null)).not.toThrow();
});

it("mmap rejects paths with interior null bytes", () => {
// The C path stops at the first NUL, so "a\0b" would silently map "a".
const evil = path + "\0.does-not-exist";
expect(() => Bun.mmap(evil)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" }));
expect(() => Bun.mmap(evil)).toThrow("must be a string without null bytes");
});

it("mmap handles non-number offset/size without crashing", () => {
// These should not crash - non-number values coerce to 0 per JavaScript semantics
// Previously these caused assertion failures (issue ENG-22413)
Expand Down
Loading