Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix path resolution for writeFile in nodefs #10179

Merged
merged 7 commits into from
Apr 15, 2024
Merged
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
50 changes: 2 additions & 48 deletions src/bun.js/node/node_fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5336,58 +5336,12 @@ pub const NodeFS = struct {
}

pub fn writeFileWithPathBuffer(pathbuf: *[bun.MAX_PATH_BYTES]u8, args: Arguments.WriteFile) Maybe(Return.WriteFile) {
var path: [:0]const u8 = undefined;
var pathbuf2: [bun.MAX_PATH_BYTES]u8 = undefined;

const fd = switch (args.file) {
.path => brk: {
// On Windows, we potentially mutate the path in posixToPlatformInPlace
// We cannot mutate JavaScript strings in-place. That will break many things.
// So we must always copy the path string on Windows.
path = path: {
const temp_path = args.file.path.sliceZWithForceCopy(pathbuf, Environment.isWindows);
if (Environment.isWindows) {
bun.path.posixToPlatformInPlace(u8, temp_path);
}
break :path temp_path;
};

var is_dirfd_different = false;
var dirfd = args.dirfd;
if (Environment.isWindows) {
while (std.mem.startsWith(u8, path, "..\\")) {
is_dirfd_different = true;
var buffer: bun.WPathBuffer = undefined;
const dirfd_path_len = std.os.windows.kernel32.GetFinalPathNameByHandleW(args.dirfd.cast(), &buffer, buffer.len, 0);
const dirfd_path = buffer[0..dirfd_path_len];
const parent_path = bun.Dirname.dirname(u16, dirfd_path).?;
if (std.mem.startsWith(u16, parent_path, &bun.windows.nt_maxpath_prefix)) @constCast(parent_path)[1] = '?';
const newdirfd = switch (bun.sys.openDirAtWindows(bun.invalid_fd, parent_path, .{ .no_follow = true })) {
.result => |fd| fd,
.err => |err| {
return .{ .err = err.withPath(path) };
},
};
path = path[3..];
dirfd = newdirfd;
}
}
defer if (is_dirfd_different) {
var d = dirfd.asDir();
d.close();
};
if (Environment.isWindows) {
// windows openat does not support path traversal, fix it here.
// use pathbuf2 here since without it 'panic: @memcpy arguments alias' triggers
if (std.mem.indexOf(u8, path, "\\.\\") != null or std.mem.indexOf(u8, path, "\\..\\") != null) {
const fixed_path = bun.path.normalizeStringWindows(path, &pathbuf2, false, false);
pathbuf2[fixed_path.len] = 0;
path = pathbuf2[0..fixed_path.len :0];
}
}
const path = args.file.path.sliceZWithForceCopy(pathbuf, true);

const open_result = Syscall.openat(
dirfd,
args.dirfd,
path,
@intFromEnum(args.flag) | os.O.NOCTTY,
args.mode,
Expand Down
11 changes: 11 additions & 0 deletions test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,17 @@ it("Dirent.name setter", () => {
expect(dirent.name).toBe("hello");
});

it("writeFileSync should correctly resolve ../..", () => {
const base = join(tmpdir(), `fs-test-${Math.random().toString(36).slice(2)}`);
const path = join(base, "foo", "bar");
mkdirSync(path, { recursive: true });
const cwd = process.cwd();
process.chdir(path);
writeFileSync("../../test.txt", "hello");
expect(readFileSync(join(base, "test.txt"), "utf8")).toBe("hello");
process.chdir(cwd);
});

it("writeFileSync in append should not truncate the file", () => {
const path = join(tmpdir(), "writeFileSync-should-not-append-" + (Date.now() * 10000).toString(16));
var str = "";
Expand Down
Loading