From 23a49dd3f7967d8f18caa01a5e1bfe7885a4134f Mon Sep 17 00:00:00 2001 From: Georgijs Vilums Date: Thu, 11 Apr 2024 13:06:52 -0700 Subject: [PATCH 1/6] fix path resolution for writeFile in nodefs --- src/bun.js/node/node_fs.zig | 50 ++----------------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 13fac010b89138..11f07db08b5466 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -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.sliceZ(pathbuf); const open_result = Syscall.openat( - dirfd, + args.dirfd, path, @intFromEnum(args.flag) | os.O.NOCTTY, args.mode, From 22aed908b1c089a634a802da3937c26983e137d4 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums Date: Thu, 11 Apr 2024 13:11:07 -0700 Subject: [PATCH 2/6] add test --- test/js/node/fs/fs.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 7b838229f7d276..f89034c6b16915 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -250,6 +250,17 @@ it("Dirent.name setter", () => { expect(dirent.name).toBe("hello"); }); +it("writeFileSync should correctly resolve ../..", () => { + const base = tmpdir(); + 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 = ""; From edbbb60ec86bc4b798379fbc4dfd84d1ebbcdb8f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 11 Apr 2024 20:12:21 +0000 Subject: [PATCH 3/6] [autofix.ci] apply automated fixes --- test/js/node/fs/fs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index f89034c6b16915..0cd0d8a2ba6b0f 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -252,7 +252,7 @@ it("Dirent.name setter", () => { it("writeFileSync should correctly resolve ../..", () => { const base = tmpdir(); - const path = join(base, "foo", "bar") + const path = join(base, "foo", "bar"); mkdirSync(path, { recursive: true }); const cwd = process.cwd(); process.chdir(path); From 7a157ad7910dc615f9371250b412ff9b0ac7e527 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums Date: Fri, 12 Apr 2024 10:25:47 -0700 Subject: [PATCH 4/6] use force copy --- src/bun.js/node/node_fs.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 11f07db08b5466..398d878e31645c 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -5338,7 +5338,7 @@ pub const NodeFS = struct { pub fn writeFileWithPathBuffer(pathbuf: *[bun.MAX_PATH_BYTES]u8, args: Arguments.WriteFile) Maybe(Return.WriteFile) { const fd = switch (args.file) { .path => brk: { - const path = args.file.path.sliceZ(pathbuf); + const path = args.file.path.sliceZWithForceCopy(pathbuf); const open_result = Syscall.openat( args.dirfd, From bb9b61a8f033f77425ec9bd9b6fb4a6666fde707 Mon Sep 17 00:00:00 2001 From: Georgijs Vilums Date: Fri, 12 Apr 2024 10:26:39 -0700 Subject: [PATCH 5/6] fix build --- src/bun.js/node/node_fs.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 398d878e31645c..137fda652c8d74 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -5338,7 +5338,7 @@ pub const NodeFS = struct { pub fn writeFileWithPathBuffer(pathbuf: *[bun.MAX_PATH_BYTES]u8, args: Arguments.WriteFile) Maybe(Return.WriteFile) { const fd = switch (args.file) { .path => brk: { - const path = args.file.path.sliceZWithForceCopy(pathbuf); + const path = args.file.path.sliceZWithForceCopy(pathbuf, true); const open_result = Syscall.openat( args.dirfd, From bec48c55aea4f3c361e825c87502ca3c89eaa26a Mon Sep 17 00:00:00 2001 From: Georgijs Vilums Date: Sun, 14 Apr 2024 16:24:58 -0700 Subject: [PATCH 6/6] fix test on windows --- test/js/node/fs/fs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 23fe37efba207b..8ff47976a44ca0 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -251,7 +251,7 @@ it("Dirent.name setter", () => { }); it("writeFileSync should correctly resolve ../..", () => { - const base = tmpdir(); + 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();