From fdcc84402722f610324b34b5994d4d3c856824a6 Mon Sep 17 00:00:00 2001 From: Georgijs <48869301+gvilums@users.noreply.github.com> Date: Mon, 15 Apr 2024 05:02:09 -0700 Subject: [PATCH] fix path resolution for writeFile in nodefs (#10179) * 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> --- src/bun.js/node/node_fs.zig | 50 ++----------------------------------- test/js/node/fs/fs.test.ts | 11 ++++++++ 2 files changed, 13 insertions(+), 48 deletions(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index a23d79a1ed8e93..2b10ba06db8402 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.sliceZWithForceCopy(pathbuf, true); const open_result = Syscall.openat( - dirfd, + args.dirfd, path, @intFromEnum(args.flag) | os.O.NOCTTY, args.mode, diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index c2b11cecd4e788..25ba2fa42558aa 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 = 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 = "";