Skip to content

Commit

Permalink
fix path resolution for writeFile in nodefs (#10179)
Browse files Browse the repository at this point in the history
* fix path resolution for writeFile in nodefs

* add test

* [autofix.ci] apply automated fixes

* use force copy

* fix build

* fix test on windows

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
gvilums and autofix-ci[bot] authored Apr 15, 2024
1 parent 74d91f6 commit fdcc844
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 48 deletions.
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

0 comments on commit fdcc844

Please sign in to comment.