From e0b77a6b77614538d18b9f0b9e3e7e434ccee4ff Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 19 Jul 2020 22:25:00 +0200 Subject: [PATCH] Ensure Dir.deleteTree does not dereference symlinks Otherwise, the behaviour can lead to unexpected results, resulting in removing an entire tree that's not necessarily under the root. Furthermore, this change is needed if are to properly handle dir symlinks on Windows. Without explicitly requiring that a directory or file is opened with `FILE_OPEN_REPARSE_POINT`, Windows automatically dereferences all symlinks along the way. This commit adds another option to `OpenDirOptions`, namely `.no_follow`, which defaults to `false` and can be used to specifically open a directory symlink on Windows or call `openat` with `O_NOFOLLOW` flag in POSIX. --- lib/std/fs.zig | 27 ++++++++++++++++++--------- lib/std/fs/test.zig | 6 +++--- lib/std/os.zig | 4 +++- lib/std/os/windows.zig | 10 +--------- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 3cdb163f38478eb33c87e3311e2190ba78e5f8cc..a22b15b0fb9aeda2c9f7a649bd5a2cef43720885 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -951,6 +951,9 @@ pub const Dir = struct { /// `true` means the opened directory can be scanned for the files and sub-directories /// of the result. It means the `iterate` function can be called. iterate: bool = false, + + /// `true` means it won't dereference the symlink. + no_follow: bool = false, }; /// Opens a directory at the given path. The directory is a system resource that remains @@ -994,10 +997,11 @@ pub const Dir = struct { w.RIGHT_PATH_REMOVE_DIRECTORY | w.RIGHT_PATH_UNLINK_FILE; } + const symlink_flags: w.lookupflags_t = if (args.no_follow) 0x0 else w.LOOKUP_SYMLINK_FOLLOW; // TODO do we really need all the rights here? const inheriting: w.rights_t = w.RIGHT_ALL ^ w.RIGHT_SOCK_SHUTDOWN; - const result = os.openatWasi(self.fd, sub_path, w.O_DIRECTORY, 0x0, base, inheriting); + const result = os.openatWasi(self.fd, sub_path, w.O_DIRECTORY, symlink_flags, base, inheriting); const fd = result catch |err| switch (err) { error.FileTooBig => unreachable, // can't happen for directories error.IsDir => unreachable, // we're providing O_DIRECTORY @@ -1014,11 +1018,13 @@ pub const Dir = struct { if (builtin.os.tag == .windows) { const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); return self.openDirW(sub_path_w.span().ptr, args); - } else if (!args.iterate) { + } + const symlink_flags: u32 = if (args.no_follow) os.O_NOFOLLOW else 0x0; + if (!args.iterate) { const O_PATH = if (@hasDecl(os, "O_PATH")) os.O_PATH else 0; - return self.openDirFlagsZ(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC | O_PATH); + return self.openDirFlagsZ(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC | O_PATH | symlink_flags); } else { - return self.openDirFlagsZ(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC); + return self.openDirFlagsZ(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC | symlink_flags); } } @@ -1030,7 +1036,7 @@ pub const Dir = struct { const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE; const flags: u32 = if (args.iterate) base_flags | w.FILE_LIST_DIRECTORY else base_flags; - return self.openDirAccessMaskW(sub_path_w, flags); + return self.openDirAccessMaskW(sub_path_w, flags, args.no_follow); } /// `flags` must contain `os.O_DIRECTORY`. @@ -1050,7 +1056,7 @@ pub const Dir = struct { return Dir{ .fd = fd }; } - fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32) OpenError!Dir { + fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32, no_follow: bool) OpenError!Dir { const w = os.windows; var result = Dir{ @@ -1080,6 +1086,7 @@ pub const Dir = struct { // implement this: https://git.midipix.org/ntapi/tree/src/fs/ntapi_tt_open_physical_parent_directory.c @panic("TODO opening '..' with a relative directory handle is not yet implemented on Windows"); } + const open_reparse_point: w.DWORD = if (no_follow) w.FILE_OPEN_REPARSE_POINT else 0x0; var io: w.IO_STATUS_BLOCK = undefined; const rc = w.ntdll.NtCreateFile( &result.fd, @@ -1090,7 +1097,7 @@ pub const Dir = struct { 0, w.FILE_SHARE_READ | w.FILE_SHARE_WRITE, w.FILE_OPEN, - w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT, + w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT | open_reparse_point, null, 0, ); @@ -1277,6 +1284,7 @@ pub const Dir = struct { pub fn deleteTree(self: Dir, sub_path: []const u8) DeleteTreeError!void { start_over: while (true) { var got_access_denied = false; + // First, try deleting the item as a file. This way we don't follow sym links. if (self.deleteFile(sub_path)) { return; @@ -1297,7 +1305,8 @@ pub const Dir = struct { error.Unexpected, => |e| return e, } - var dir = self.openDir(sub_path, .{ .iterate = true }) catch |err| switch (err) { + + var dir = self.openDir(sub_path, .{ .iterate = true, .no_follow = true }) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; @@ -1364,7 +1373,7 @@ pub const Dir = struct { => |e| return e, } - const new_dir = dir.openDir(entry.name, .{ .iterate = true }) catch |err| switch (err) { + const new_dir = dir.openDir(entry.name, .{ .iterate = true, .no_follow = true }) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index c20cb2e62fdfcfd6933e5530c717277708599b22..4fecf3c9c539359adb89421331e79d4bd10a38bd 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -36,7 +36,7 @@ test "readLinkAbsolute" { // Create symbolic link by path try fs.symLinkAbsolute(target_path, symlink_path, .{}); - try testReadlinkAbsolute(target_path, symlink_path); + try testReadLinkAbsolute(target_path, symlink_path); } { const target_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "subdir" }); @@ -44,11 +44,11 @@ test "readLinkAbsolute" { // Create symbolic link by path try fs.symLinkAbsolute(target_path, symlink_path, .{ .is_directory = true }); - try testReadlinkAbsolute(target_path, symlink_path); + try testReadLinkAbsolute(target_path, symlink_path); } } -fn testReadlinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void { +fn testReadLinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void { var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; const given = try fs.readLinkAbsolute(symlink_path, buffer[0..]); testing.expect(mem.eql(u8, target_path, given)); diff --git a/lib/std/os.zig b/lib/std/os.zig index ebe457fe4d3b9135236697fc2bbad647c1ee5b1c..86a5a7ad08349527d849e63f0f78ad3d71027798 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1815,7 +1815,7 @@ pub fn unlinkatW(dirfd: fd_t, sub_path_w: [*:0]const u16, flags: u32) UnlinkatEr const want_rmdir_behavior = (flags & AT_REMOVEDIR) != 0; const create_options_flags = if (want_rmdir_behavior) - @as(w.ULONG, w.FILE_DELETE_ON_CLOSE | w.FILE_DIRECTORY_FILE) + @as(w.ULONG, w.FILE_DELETE_ON_CLOSE | w.FILE_DIRECTORY_FILE | w.FILE_OPEN_REPARSE_POINT) else @as(w.ULONG, w.FILE_DELETE_ON_CLOSE | w.FILE_NON_DIRECTORY_FILE | w.FILE_OPEN_REPARSE_POINT); // would we ever want to delete the target instead? @@ -2390,9 +2390,11 @@ pub fn readlinkW(file_path: [*:0]const u16, out_buffer: []u8) ReadLinkError![]u8 else => |e| return e, } }; + defer w.CloseHandle(handle); var reparse_buf: [w.MAXIMUM_REPARSE_DATA_BUFFER_SIZE]u8 = undefined; _ = try w.DeviceIoControl(handle, w.FSCTL_GET_REPARSE_POINT, null, reparse_buf[0..], null); + const reparse_struct = @ptrCast(*const w.REPARSE_DATA_BUFFER, @alignCast(@alignOf(w.REPARSE_DATA_BUFFER), &reparse_buf[0])); switch (reparse_struct.ReparseTag) { w.IO_REPARSE_TAG_SYMLINK => { diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 7835742e39dbb1bcaf96a2d0da9b8bd90b435433..3895e5d80a78c7e0b074c7bdc13f7dac300e91ba 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -602,15 +602,7 @@ pub fn GetCurrentDirectory(buffer: []u8) GetCurrentDirectoryError![]u8 { return buffer[0..end_index]; } -pub const CreateSymbolicLinkError = error{ - AccessDenied, - PathAlreadyExists, - FileNotFound, - NameTooLong, - InvalidUtf8, - BadPathName, - Unexpected -}; +pub const CreateSymbolicLinkError = error{ AccessDenied, PathAlreadyExists, FileNotFound, NameTooLong, InvalidUtf8, BadPathName, Unexpected }; pub fn CreateSymbolicLink( sym_link_path: []const u8, -- 2.54.0