From 5ab67bff1e2230217ce133165605f8dc0d588178 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 6 Jan 2022 02:47:36 +0000 Subject: [PATCH 1/5] Fix CVE-2022-21658 for Windows --- library/std/src/sys/windows/c.rs | 124 +++++++++++- library/std/src/sys/windows/fs.rs | 322 ++++++++++++++++++++++++++++-- 2 files changed, 419 insertions(+), 27 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index b87b6b5d88e4a..09d3661e4fd52 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -4,6 +4,7 @@ #![cfg_attr(test, allow(dead_code))] #![unstable(issue = "none", feature = "windows_c")] +use crate::mem; use crate::os::raw::NonZero_c_ulong; use crate::os::raw::{c_char, c_int, c_long, c_longlong, c_uint, c_ulong, c_ushort}; use crate::ptr; @@ -36,6 +37,7 @@ pub type USHORT = c_ushort; pub type SIZE_T = usize; pub type WORD = u16; pub type CHAR = c_char; +pub type CCHAR = c_char; pub type ULONG_PTR = usize; pub type ULONG = c_ulong; pub type NTSTATUS = LONG; @@ -86,16 +88,21 @@ pub const FILE_SHARE_DELETE: DWORD = 0x4; pub const FILE_SHARE_READ: DWORD = 0x1; pub const FILE_SHARE_WRITE: DWORD = 0x2; +pub const FILE_OPEN_REPARSE_POINT: ULONG = 0x200000; +pub const OBJ_DONT_REPARSE: ULONG = 0x1000; + pub const CREATE_ALWAYS: DWORD = 2; pub const CREATE_NEW: DWORD = 1; pub const OPEN_ALWAYS: DWORD = 4; pub const OPEN_EXISTING: DWORD = 3; pub const TRUNCATE_EXISTING: DWORD = 5; +pub const FILE_LIST_DIRECTORY: DWORD = 0x1; pub const FILE_WRITE_DATA: DWORD = 0x00000002; pub const FILE_APPEND_DATA: DWORD = 0x00000004; pub const FILE_WRITE_EA: DWORD = 0x00000010; pub const FILE_WRITE_ATTRIBUTES: DWORD = 0x00000100; +pub const DELETE: DWORD = 0x10000; pub const READ_CONTROL: DWORD = 0x00020000; pub const SYNCHRONIZE: DWORD = 0x00100000; pub const GENERIC_READ: DWORD = 0x80000000; @@ -261,9 +268,61 @@ pub const FD_SETSIZE: usize = 64; pub const STACK_SIZE_PARAM_IS_A_RESERVATION: DWORD = 0x00010000; pub const STATUS_SUCCESS: NTSTATUS = 0x00000000; +pub const STATUS_DELETE_PENDING: NTSTATUS = 0xc0000056_u32 as _; +pub const STATUS_INVALID_PARAMETER: NTSTATUS = 0xc000000d_u32 as _; + +// Equivalent to the `NT_SUCCESS` C preprocessor macro. +// See: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/using-ntstatus-values +pub fn nt_success(status: NTSTATUS) -> bool { + status >= 0 +} pub const BCRYPT_USE_SYSTEM_PREFERRED_RNG: DWORD = 0x00000002; +#[repr(C)] +pub struct UNICODE_STRING { + pub Length: u16, + pub MaximumLength: u16, + pub Buffer: *mut u16, +} +impl UNICODE_STRING { + pub fn from_ref(slice: &[u16]) -> Self { + let len = slice.len() * mem::size_of::(); + Self { Length: len as _, MaximumLength: len as _, Buffer: slice.as_ptr() as _ } + } +} +#[repr(C)] +pub struct OBJECT_ATTRIBUTES { + pub Length: ULONG, + pub RootDirectory: HANDLE, + pub ObjectName: *const UNICODE_STRING, + pub Attributes: ULONG, + pub SecurityDescriptor: *mut c_void, + pub SecurityQualityOfService: *mut c_void, +} +impl Default for OBJECT_ATTRIBUTES { + fn default() -> Self { + Self { + Length: mem::size_of::() as _, + RootDirectory: ptr::null_mut(), + ObjectName: ptr::null_mut(), + Attributes: 0, + SecurityDescriptor: ptr::null_mut(), + SecurityQualityOfService: ptr::null_mut(), + } + } +} +#[repr(C)] +pub struct IO_STATUS_BLOCK { + pub Pointer: *mut c_void, + pub Information: usize, +} +impl Default for IO_STATUS_BLOCK { + fn default() -> Self { + Self { Pointer: ptr::null_mut(), Information: 0 } + } +} + #[repr(C)] #[cfg(not(target_pointer_width = "64"))] pub struct WSADATA { @@ -353,9 +412,43 @@ pub enum FILE_INFO_BY_HANDLE_CLASS { FileIdInfo = 18, // 0x12 FileIdExtdDirectoryInfo = 19, // 0x13 FileIdExtdDirectoryRestartInfo = 20, // 0x14 + FileDispositionInfoEx = 21, // 0x15, Windows 10 version 1607 MaximumFileInfoByHandlesClass, } +#[repr(C)] +pub struct FILE_DISPOSITION_INFO { + pub DeleteFile: BOOLEAN, +} + +pub const FILE_DISPOSITION_DELETE: DWORD = 0x1; +pub const FILE_DISPOSITION_POSIX_SEMANTICS: DWORD = 0x2; +pub const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: DWORD = 0x10; + +#[repr(C)] +pub struct FILE_DISPOSITION_INFO_EX { + pub Flags: DWORD, +} + +#[repr(C)] +#[derive(Default)] +pub struct FILE_ID_BOTH_DIR_INFO { + pub NextEntryOffset: DWORD, + pub FileIndex: DWORD, + pub CreationTime: LARGE_INTEGER, + pub LastAccessTime: LARGE_INTEGER, + pub LastWriteTime: LARGE_INTEGER, + pub ChangeTime: LARGE_INTEGER, + pub EndOfFile: LARGE_INTEGER, + pub AllocationSize: LARGE_INTEGER, + pub FileAttributes: DWORD, + pub FileNameLength: DWORD, + pub EaSize: DWORD, + pub ShortNameLength: CCHAR, + pub ShortName: [WCHAR; 12], + pub FileId: LARGE_INTEGER, + pub FileName: [WCHAR; 1], +} #[repr(C)] pub struct FILE_BASIC_INFO { pub CreationTime: LARGE_INTEGER, @@ -750,16 +843,6 @@ if #[cfg(target_vendor = "uwp")] { pub DeletePending: BOOLEAN, pub Directory: BOOLEAN, } - - #[link(name = "kernel32")] - extern "system" { - pub fn GetFileInformationByHandleEx( - hFile: HANDLE, - fileInfoClass: FILE_INFO_BY_HANDLE_CLASS, - lpFileInformation: LPVOID, - dwBufferSize: DWORD, - ) -> BOOL; - } } } @@ -949,6 +1032,12 @@ extern "system" { cchFilePath: DWORD, dwFlags: DWORD, ) -> DWORD; + pub fn GetFileInformationByHandleEx( + hFile: HANDLE, + fileInfoClass: FILE_INFO_BY_HANDLE_CLASS, + lpFileInformation: LPVOID, + dwBufferSize: DWORD, + ) -> BOOL; pub fn SetFileInformationByHandle( hFile: HANDLE, FileInformationClass: FILE_INFO_BY_HANDLE_CLASS, @@ -1139,6 +1228,21 @@ compat_fn! { compat_fn! { "ntdll": + pub fn NtOpenFile( + FileHandle: *mut HANDLE, + DesiredAccess: ACCESS_MASK, + ObjectAttributes: *const OBJECT_ATTRIBUTES, + IoStatusBlock: *mut IO_STATUS_BLOCK, + ShareAccess: ULONG, + OpenOptions: ULONG + ) -> NTSTATUS { + panic!("`NtOpenFile` not available"); + } + pub fn RtlNtStatusToDosError( + Status: NTSTATUS + ) -> ULONG { + panic!("`RtlNtStatusToDosError` not available"); + } pub fn NtCreateKeyedEvent( KeyedEventHandle: LPHANDLE, DesiredAccess: ACCESS_MASK, diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index b258fc0478bdc..dd21c6b43891f 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -547,6 +547,218 @@ impl File { })?; Ok(()) } + /// Get only basic file information such as attributes and file times. + fn basic_info(&self) -> io::Result { + unsafe { + let mut info: c::FILE_BASIC_INFO = mem::zeroed(); + let size = mem::size_of_val(&info); + cvt(c::GetFileInformationByHandleEx( + self.handle.as_raw_handle(), + c::FileBasicInfo, + &mut info as *mut _ as *mut libc::c_void, + size as c::DWORD, + ))?; + Ok(info) + } + } + /// Delete using POSIX semantics. + /// + /// Files will be deleted as soon as the handle is closed. This is supported + /// for Windows 10 1607 (aka RS1) and later. However some filesystem + /// drivers will not support it even then, e.g. FAT32. + /// + /// If the operation is not supported for this filesystem or OS version + /// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`. + fn posix_delete(&self) -> io::Result<()> { + let mut info = c::FILE_DISPOSITION_INFO_EX { + Flags: c::FILE_DISPOSITION_DELETE + | c::FILE_DISPOSITION_POSIX_SEMANTICS + | c::FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE, + }; + let size = mem::size_of_val(&info); + cvt(unsafe { + c::SetFileInformationByHandle( + self.handle.as_raw_handle(), + c::FileDispositionInfoEx, + &mut info as *mut _ as *mut _, + size as c::DWORD, + ) + })?; + Ok(()) + } + + /// Delete a file using win32 semantics. The file won't actually be deleted + /// until all file handles are closed. However, marking a file for deletion + /// will prevent anyone from opening a new handle to the file. + fn win32_delete(&self) -> io::Result<()> { + let mut info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ }; + let size = mem::size_of_val(&info); + cvt(unsafe { + c::SetFileInformationByHandle( + self.handle.as_raw_handle(), + c::FileDispositionInfo, + &mut info as *mut _ as *mut _, + size as c::DWORD, + ) + })?; + Ok(()) + } + + /// Fill the given buffer with as many directory entries as will fit. + /// This will remember its position and continue from the last call unless + /// `restart` is set to `true`. + /// + /// The returned bool indicates if there are more entries or not. + /// It is an error if `self` is not a directory. + /// + /// # Symlinks and other reparse points + /// + /// On Windows a file is either a directory or a non-directory. + /// A symlink directory is simply an empty directory with some "reparse" metadata attached. + /// So if you open a link (not its target) and iterate the directory, + /// you will always iterate an empty directory regardless of the target. + fn fill_dir_buff(&self, buffer: &mut DirBuff, restart: bool) -> io::Result { + let class = + if restart { c::FileIdBothDirectoryRestartInfo } else { c::FileIdBothDirectoryInfo }; + + unsafe { + let result = cvt(c::GetFileInformationByHandleEx( + self.handle.as_raw_handle(), + class, + buffer.as_mut_ptr().cast(), + buffer.capacity() as _, + )); + match result { + Ok(_) => Ok(true), + Err(e) if e.raw_os_error() == Some(c::ERROR_NO_MORE_FILES as _) => Ok(false), + Err(e) => Err(e), + } + } + } +} + +/// A buffer for holding directory entries. +struct DirBuff { + buffer: Vec, +} +impl DirBuff { + fn new() -> Self { + const BUFFER_SIZE: usize = 1024; + Self { buffer: vec![0_u8; BUFFER_SIZE] } + } + fn capacity(&self) -> usize { + self.buffer.len() + } + fn as_mut_ptr(&mut self) -> *mut u8 { + self.buffer.as_mut_ptr().cast() + } + /// Returns a `DirBuffIter`. + fn iter(&self) -> DirBuffIter<'_> { + DirBuffIter::new(self) + } +} +impl AsRef<[u8]> for DirBuff { + fn as_ref(&self) -> &[u8] { + &self.buffer + } +} + +/// An iterator over entries stored in a `DirBuff`. +/// +/// Currently only returns file names (UTF-16 encoded). +struct DirBuffIter<'a> { + buffer: Option<&'a [u8]>, + cursor: usize, +} +impl<'a> DirBuffIter<'a> { + fn new(buffer: &'a DirBuff) -> Self { + Self { buffer: Some(buffer.as_ref()), cursor: 0 } + } +} +impl<'a> Iterator for DirBuffIter<'a> { + type Item = &'a [u16]; + fn next(&mut self) -> Option { + use crate::mem::size_of; + let buffer = &self.buffer?[self.cursor..]; + + // Get the name and next entry from the buffer. + // SAFETY: The buffer contains a `FILE_ID_BOTH_DIR_INFO` struct but the + // last field (the file name) is unsized. So an offset has to be + // used to get the file name slice. + let (name, next_entry) = unsafe { + let info = buffer.as_ptr().cast::(); + let next_entry = (*info).NextEntryOffset as usize; + let name = crate::slice::from_raw_parts( + (*info).FileName.as_ptr().cast::(), + (*info).FileNameLength as usize / size_of::(), + ); + (name, next_entry) + }; + + if next_entry == 0 { + self.buffer = None + } else { + self.cursor += next_entry + } + + // Skip `.` and `..` pseudo entries. + const DOT: u16 = b'.' as u16; + match name { + [DOT] | [DOT, DOT] => self.next(), + _ => Some(name), + } + } +} + +/// Open a link relative to the parent directory, ensure no symlinks are followed. +fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result { + // This is implemented using the lower level `NtOpenFile` function as + // unfortunately opening a file relative to a parent is not supported by + // win32 functions. It is however a fundamental feature of the NT kernel. + // + // See https://docs.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntopenfile + unsafe { + let mut handle = ptr::null_mut(); + let mut io_status = c::IO_STATUS_BLOCK::default(); + let name_str = c::UNICODE_STRING::from_ref(name); + use crate::sync::atomic::{AtomicU32, Ordering}; + // The `OBJ_DONT_REPARSE` attribute ensures that we haven't been + // tricked into following a symlink. However, it may not be available in + // earlier versions of Windows. + static ATTRIBUTES: AtomicU32 = AtomicU32::new(c::OBJ_DONT_REPARSE); + let object = c::OBJECT_ATTRIBUTES { + ObjectName: &name_str, + RootDirectory: parent.as_raw_handle(), + Attributes: ATTRIBUTES.load(Ordering::Relaxed), + ..c::OBJECT_ATTRIBUTES::default() + }; + let status = c::NtOpenFile( + &mut handle, + access, + &object, + &mut io_status, + c::FILE_SHARE_DELETE | c::FILE_SHARE_READ | c::FILE_SHARE_WRITE, + // If `name` is a symlink then open the link rather than the target. + c::FILE_OPEN_REPARSE_POINT, + ); + // Convert an NTSTATUS to the more familiar Win32 error codes (aka "DosError") + if c::nt_success(status) { + Ok(File::from_raw_handle(handle)) + } else if status == c::STATUS_DELETE_PENDING { + // We make a special exception for `STATUS_DELETE_PENDING` because + // otherwise this will be mapped to `ERROR_ACCESS_DENIED` which is + // very unhelpful. + Err(io::Error::from_raw_os_error(c::ERROR_DELETE_PENDING as _)) + } else if status == c::STATUS_INVALID_PARAMETER + && ATTRIBUTES.load(Ordering::Relaxed) == c::OBJ_DONT_REPARSE + { + // Try without `OBJ_DONT_REPARSE`. See above. + ATTRIBUTES.store(0, Ordering::Relaxed); + open_link_no_reparse(parent, name, access) + } else { + Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as _)) + } + } } impl AsInner for File { @@ -756,30 +968,106 @@ pub fn rmdir(p: &Path) -> io::Result<()> { Ok(()) } +/// Open a file or directory without following symlinks. +fn open_link(path: &Path, access_mode: u32) -> io::Result { + let mut opts = OpenOptions::new(); + opts.access_mode(access_mode); + // `FILE_FLAG_BACKUP_SEMANTICS` allows opening directories. + // `FILE_FLAG_OPEN_REPARSE_POINT` opens a link instead of its target. + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | c::FILE_FLAG_OPEN_REPARSE_POINT); + File::open(path, &opts) +} + pub fn remove_dir_all(path: &Path) -> io::Result<()> { - let filetype = lstat(path)?.file_type(); - if filetype.is_symlink() { - // On Windows symlinks to files and directories are removed differently. - // rmdir only deletes dir symlinks and junctions, not file symlinks. - rmdir(path) + let file = open_link(path, c::DELETE | c::FILE_LIST_DIRECTORY)?; + + // Test if the file is not a directory or a symlink to a directory. + if (file.basic_info()?.FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) == 0 { + return Err(io::Error::from_raw_os_error(c::ERROR_DIRECTORY as _)); + } + let mut delete: fn(&File) -> io::Result<()> = File::posix_delete; + let result = match delete(&file) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + match remove_dir_all_recursive(&file, delete) { + // Return unexpected errors. + Err(e) if e.kind() != io::ErrorKind::DirectoryNotEmpty => return Err(e), + result => result, + } + } + // If POSIX delete is not supported for this filesystem then fallback to win32 delete. + Err(e) + if e.raw_os_error() == Some(c::ERROR_NOT_SUPPORTED as i32) + || e.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as i32) => + { + delete = File::win32_delete; + Err(e) + } + result => result, + }; + if result.is_ok() { + Ok(()) } else { - remove_dir_all_recursive(path) + // This is a fallback to make sure the directory is actually deleted. + // Otherwise this function is prone to failing with `DirectoryNotEmpty` + // due to possible delays between marking a file for deletion and the + // file actually being deleted from the filesystem. + // + // So we retry a few times before giving up. + for _ in 0..5 { + match remove_dir_all_recursive(&file, delete) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => {} + result => return result, + } + } + // Try one last time. + delete(&file) } } -fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { - for child in readdir(path)? { - let child = child?; - let child_type = child.file_type()?; - if child_type.is_dir() { - remove_dir_all_recursive(&child.path())?; - } else if child_type.is_symlink_dir() { - rmdir(&child.path())?; - } else { - unlink(&child.path())?; +fn remove_dir_all_recursive(f: &File, delete: fn(&File) -> io::Result<()>) -> io::Result<()> { + let mut buffer = DirBuff::new(); + let mut restart = true; + // Fill the buffer and iterate the entries. + while f.fill_dir_buff(&mut buffer, restart)? { + for name in buffer.iter() { + // Open the file without following symlinks and try deleting it. + // We try opening will all needed permissions and if that is denied + // fallback to opening without `FILE_LIST_DIRECTORY` permission. + // Note `SYNCHRONIZE` permission is needed for synchronous access. + let mut result = + open_link_no_reparse(&f, name, c::SYNCHRONIZE | c::DELETE | c::FILE_LIST_DIRECTORY); + if matches!(&result, Err(e) if e.kind() == io::ErrorKind::PermissionDenied) { + result = open_link_no_reparse(&f, name, c::SYNCHRONIZE | c::DELETE); + } + match result { + Ok(file) => match delete(&file) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + // Iterate the directory's files. + // Ignore `DirectoryNotEmpty` errors here. They will be + // caught when `remove_dir_all` tries to delete the top + // level directory. It can then decide if to retry or not. + match remove_dir_all_recursive(&file, delete) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => {} + result => result?, + } + } + result => result?, + }, + // Ignore error if a delete is already in progress or the file + // has already been deleted. It also ignores sharing violations + // (where a file is locked by another process) as these are + // usually temporary. + Err(e) + if e.raw_os_error() == Some(c::ERROR_DELETE_PENDING as _) + || e.kind() == io::ErrorKind::NotFound + || e.raw_os_error() == Some(c::ERROR_SHARING_VIOLATION as _) => {} + Err(e) => return Err(e), + } } + // Continue reading directory entries without restarting from the beginning, + restart = false; } - rmdir(path) + delete(&f) } pub fn readlink(path: &Path) -> io::Result { From 54e22eb7dbb615bd44355028d3fd867aa93c0972 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Sat, 18 Dec 2021 15:30:30 +0100 Subject: [PATCH 2/5] Fix CVE-2022-21658 for UNIX-like --- library/std/src/fs/tests.rs | 70 ++++++++ library/std/src/sys/unix/fs.rs | 288 +++++++++++++++++++++++++++++-- library/std/src/sys/unix/weak.rs | 6 +- 3 files changed, 351 insertions(+), 13 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 749d51d4981c3..a62c01ef29b27 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -4,8 +4,10 @@ use crate::fs::{self, File, OpenOptions}; use crate::io::{ErrorKind, SeekFrom}; use crate::path::Path; use crate::str; +use crate::sync::Arc; use crate::sys_common::io::test::{tmpdir, TempDir}; use crate::thread; +use crate::time::{Duration, Instant}; use rand::{rngs::StdRng, RngCore, SeedableRng}; @@ -601,6 +603,21 @@ fn recursive_rmdir_of_symlink() { assert!(canary.exists()); } +#[test] +fn recursive_rmdir_of_file_fails() { + // test we do not delete a directly specified file. + let tmpdir = tmpdir(); + let canary = tmpdir.join("do_not_delete"); + check!(check!(File::create(&canary)).write(b"foo")); + let result = fs::remove_dir_all(&canary); + #[cfg(unix)] + error!(result, "Not a directory"); + #[cfg(windows)] + error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid. + assert!(result.is_err()); + assert!(canary.exists()); +} + #[test] // only Windows makes a distinction between file and directory symlinks. #[cfg(windows)] @@ -620,6 +637,59 @@ fn recursive_rmdir_of_file_symlink() { } } +#[test] +#[ignore] // takes too much time +fn recursive_rmdir_toctou() { + // Test for time-of-check to time-of-use issues. + // + // Scenario: + // The attacker wants to get directory contents deleted, to which he does not have access. + // He has a way to get a privileged Rust binary call `std::fs::remove_dir_all()` on a + // directory he controls, e.g. in his home directory. + // + // The POC sets up the `attack_dest/attack_file` which the attacker wants to have deleted. + // The attacker repeatedly creates a directory and replaces it with a symlink from + // `victim_del` to `attack_dest` while the victim code calls `std::fs::remove_dir_all()` + // on `victim_del`. After a few seconds the attack has succeeded and + // `attack_dest/attack_file` is deleted. + let tmpdir = tmpdir(); + let victim_del_path = tmpdir.join("victim_del"); + let victim_del_path_clone = victim_del_path.clone(); + + // setup dest + let attack_dest_dir = tmpdir.join("attack_dest"); + let attack_dest_dir = attack_dest_dir.as_path(); + fs::create_dir(attack_dest_dir).unwrap(); + let attack_dest_file = tmpdir.join("attack_dest/attack_file"); + File::create(&attack_dest_file).unwrap(); + + let drop_canary_arc = Arc::new(()); + let drop_canary_weak = Arc::downgrade(&drop_canary_arc); + + eprintln!("x: {:?}", &victim_del_path); + + // victim just continuously removes `victim_del` + thread::spawn(move || { + while drop_canary_weak.upgrade().is_some() { + let _ = fs::remove_dir_all(&victim_del_path_clone); + } + }); + + // attacker (could of course be in a separate process) + let start_time = Instant::now(); + while Instant::now().duration_since(start_time) < Duration::from_secs(1000) { + if !attack_dest_file.exists() { + panic!( + "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.", + Instant::now().duration_since(start_time) + ); + } + let _ = fs::create_dir(&victim_del_path); + let _ = fs::remove_dir(&victim_del_path); + let _ = symlink_dir(attack_dest_dir, &victim_del_path); + } +} + #[test] fn unicode_path_is_dir() { assert!(Path::new(".").is_dir()); diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index bcf2be0e95fb7..16460bee66ff2 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -64,7 +64,7 @@ use libc::{ dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, readdir64_r, stat64, }; -pub use crate::sys_common::fs::{remove_dir_all, try_exists}; +pub use crate::sys_common::fs::try_exists; pub struct File(FileDesc); @@ -228,7 +228,7 @@ pub struct DirEntry { target_os = "fuchsia", target_os = "redox" ))] - name: Box<[u8]>, + name: CString, } #[derive(Clone, Debug)] @@ -455,8 +455,6 @@ impl Iterator for ReadDir { target_os = "illumos" ))] fn next(&mut self) -> Option> { - use crate::slice; - unsafe { loop { // Although readdir_r(3) would be a correct function to use here because @@ -474,14 +472,10 @@ impl Iterator for ReadDir { }; } - let name = (*entry_ptr).d_name.as_ptr(); - let namelen = libc::strlen(name) as usize; - let ret = DirEntry { entry: *entry_ptr, - name: slice::from_raw_parts(name as *const u8, namelen as usize) - .to_owned() - .into_boxed_slice(), + // d_name is guaranteed to be null-terminated. + name: CStr::from_ptr((*entry_ptr).d_name.as_ptr()).to_owned(), dir: Arc::clone(&self.inner), }; if ret.name_bytes() != b"." && ret.name_bytes() != b".." { @@ -664,7 +658,26 @@ impl DirEntry { target_os = "redox" ))] fn name_bytes(&self) -> &[u8] { - &*self.name + self.name.as_bytes() + } + + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox" + )))] + fn name_cstr(&self) -> &CStr { + unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } + } + #[cfg(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox" + ))] + fn name_cstr(&self) -> &CStr { + &self.name } pub fn file_name_os_str(&self) -> &OsStr { @@ -1437,3 +1450,256 @@ pub fn chroot(dir: &Path) -> io::Result<()> { cvt(unsafe { libc::chroot(dir.as_ptr()) })?; Ok(()) } + +pub use remove_dir_impl::remove_dir_all; + +// Fallback for REDOX +#[cfg(target_os = "redox")] +mod remove_dir_impl { + pub use crate::sys_common::fs::remove_dir_all; +} + +// Dynamically choose implementation Macos x86-64: modern for 10.10+, fallback for older versions +#[cfg(all(target_os = "macos", target_arch = "x86_64"))] +mod remove_dir_impl { + use super::{cstr, lstat, Dir, InnerReadDir, ReadDir}; + use crate::ffi::CStr; + use crate::io; + use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; + use crate::os::unix::prelude::{OwnedFd, RawFd}; + use crate::path::{Path, PathBuf}; + use crate::sync::Arc; + use crate::sys::weak::weak; + use crate::sys::{cvt, cvt_r}; + use libc::{c_char, c_int, DIR}; + + pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + let fd = cvt_r(|| unsafe { + openat.get().unwrap()( + parent_fd.unwrap_or(libc::AT_FDCWD), + p.as_ptr(), + libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY, + ) + })?; + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) + } + + fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> { + weak!(fn fdopendir(c_int) -> *mut DIR, "fdopendir$INODE64"); + let ptr = unsafe { fdopendir.get().unwrap()(dir_fd.as_raw_fd()) }; + if ptr.is_null() { + return Err(io::Error::last_os_error()); + } + let dirp = Dir(ptr); + // file descriptor is automatically closed by libc::closedir() now, so give up ownership + let new_parent_fd = dir_fd.into_raw_fd(); + // a valid root is not needed because we do not call any functions involving the full path + // of the DirEntrys. + let dummy_root = PathBuf::new(); + Ok(( + ReadDir { + inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), + end_of_stream: false, + }, + new_parent_fd, + )) + } + + fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { + weak!(fn unlinkat(c_int, *const c_char, c_int) -> c_int); + + let pcstr = cstr(p)?; + + // entry is expected to be a directory, open as such + let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; + + // open the directory passing ownership of the fd + let (dir, fd) = fdreaddir(fd)?; + for child in dir { + let child = child?; + match child.entry.d_type { + libc::DT_DIR => { + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + libc::DT_UNKNOWN => { + match cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) }) + { + // type unknown - try to unlink + Err(err) if err.raw_os_error() == Some(libc::EPERM) => { + // if the file is a directory unlink fails with EPERM + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + result => { + result?; + } + } + } + _ => { + // not a directory -> unlink + cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) })?; + } + } + } + + // unlink the directory after removing its contents + cvt(unsafe { + unlinkat.get().unwrap()( + parent_fd.unwrap_or(libc::AT_FDCWD), + pcstr.as_ptr(), + libc::AT_REMOVEDIR, + ) + })?; + Ok(()) + } + + fn remove_dir_all_modern(p: &Path) -> io::Result<()> { + // We cannot just call remove_dir_all_recursive() here because that would not delete a passed + // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse + // into symlinks. + let attr = lstat(p)?; + if attr.file_type().is_symlink() { + crate::fs::remove_file(p) + } else { + remove_dir_all_recursive(None, p) + } + } + + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + if openat.get().is_some() { + // openat() is available with macOS 10.10+, just like unlinkat() and fdopendir() + remove_dir_all_modern(p) + } else { + // fall back to classic implementation + crate::sys_common::fs::remove_dir_all(p) + } + } +} + +// Modern implementation using openat(), unlinkat() and fdopendir() +#[cfg(not(any(all(target_os = "macos", target_arch = "x86_64"), target_os = "redox")))] +mod remove_dir_impl { + use super::{cstr, lstat, Dir, DirEntry, InnerReadDir, ReadDir}; + use crate::ffi::CStr; + use crate::io; + use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; + use crate::os::unix::prelude::{OwnedFd, RawFd}; + use crate::path::{Path, PathBuf}; + use crate::sync::Arc; + use crate::sys::{cvt, cvt_r}; + use libc::{fdopendir, openat, unlinkat}; + + pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { + let fd = cvt_r(|| unsafe { + openat( + parent_fd.unwrap_or(libc::AT_FDCWD), + p.as_ptr(), + libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY, + ) + })?; + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) + } + + fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> { + let ptr = unsafe { fdopendir(dir_fd.as_raw_fd()) }; + if ptr.is_null() { + return Err(io::Error::last_os_error()); + } + let dirp = Dir(ptr); + // file descriptor is automatically closed by libc::closedir() now, so give up ownership + let new_parent_fd = dir_fd.into_raw_fd(); + // a valid root is not needed because we do not call any functions involving the full path + // of the DirEntrys. + let dummy_root = PathBuf::new(); + Ok(( + ReadDir { + inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox", + )))] + end_of_stream: false, + }, + new_parent_fd, + )) + } + + #[cfg(any( + target_os = "solaris", + target_os = "illumos", + target_os = "haiku", + target_os = "vxworks" + ))] + fn is_dir(_ent: &DirEntry) -> Option { + None + } + + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "haiku", + target_os = "vxworks" + )))] + fn is_dir(ent: &DirEntry) -> Option { + match ent.entry.d_type { + libc::DT_UNKNOWN => None, + libc::DT_DIR => Some(true), + _ => Some(false), + } + } + + fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { + let pcstr = cstr(p)?; + + // entry is expected to be a directory, open as such + let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; + + // open the directory passing ownership of the fd + let (dir, fd) = fdreaddir(fd)?; + for child in dir { + let child = child?; + match is_dir(&child) { + Some(true) => { + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + Some(false) => { + cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?; + } + None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) { + // type unknown - try to unlink + Err(err) + if err.raw_os_error() == Some(libc::EISDIR) + || err.raw_os_error() == Some(libc::EPERM) => + { + // if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + result => { + result?; + } + }, + } + } + + // unlink the directory after removing its contents + cvt(unsafe { + unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), pcstr.as_ptr(), libc::AT_REMOVEDIR) + })?; + Ok(()) + } + + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + // We cannot just call remove_dir_all_recursive() here because that would not delete a passed + // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse + // into symlinks. + let attr = lstat(p)?; + if attr.file_type().is_symlink() { + crate::fs::remove_file(p) + } else { + remove_dir_all_recursive(None, p) + } + } +} diff --git a/library/std/src/sys/unix/weak.rs b/library/std/src/sys/unix/weak.rs index 55719b87c7e06..da63c068384a2 100644 --- a/library/std/src/sys/unix/weak.rs +++ b/library/std/src/sys/unix/weak.rs @@ -73,12 +73,14 @@ impl ExternWeak { pub(crate) macro dlsym { (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + dlsym!(fn $name($($t),*) -> $ret, stringify!($name)); + ), + (fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => ( static DLSYM: DlsymWeak $ret> = - DlsymWeak::new(concat!(stringify!($name), '\0')); + DlsymWeak::new(concat!($sym, '\0')); let $name = &DLSYM; ) } - pub(crate) struct DlsymWeak { name: &'static str, addr: AtomicUsize, From cb748a27d28df86cfe036709dc8092896abde31a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Jan 2022 08:44:15 -0800 Subject: [PATCH 3/5] Fix CVE-2022-21658 for WASI --- library/std/src/sys/wasi/fs.rs | 71 ++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index 1a3da3746ac61..5924789d12ba4 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -16,7 +16,7 @@ use crate::sys::time::SystemTime; use crate::sys::unsupported; use crate::sys_common::{AsInner, FromInner, IntoInner}; -pub use crate::sys_common::fs::{remove_dir_all, try_exists}; +pub use crate::sys_common::fs::try_exists; pub struct File { fd: WasiFd, @@ -130,6 +130,18 @@ impl FileType { } } +impl ReadDir { + fn new(dir: File, root: PathBuf) -> ReadDir { + ReadDir { + cookie: Some(0), + buf: vec![0; 128], + offset: 0, + cap: 0, + inner: Arc::new(ReadDirInner { dir, root }), + } + } +} + impl fmt::Debug for ReadDir { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ReadDir").finish_non_exhaustive() @@ -516,13 +528,7 @@ pub fn readdir(p: &Path) -> io::Result { opts.directory(true); opts.read(true); let dir = File::open(p, &opts)?; - Ok(ReadDir { - cookie: Some(0), - buf: vec![0; 128], - offset: 0, - cap: 0, - inner: Arc::new(ReadDirInner { dir, root: p.to_path_buf() }), - }) + Ok(ReadDir::new(dir, p.to_path_buf())) } pub fn unlink(p: &Path) -> io::Result<()> { @@ -716,3 +722,52 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { io::copy(&mut reader, &mut writer) } + +pub fn remove_dir_all(path: &Path) -> io::Result<()> { + let (parent, path) = open_parent(path)?; + remove_dir_all_recursive(&parent, &path) +} + +fn remove_dir_all_recursive(parent: &WasiFd, path: &Path) -> io::Result<()> { + // Open up a file descriptor for the directory itself. Note that we don't + // follow symlinks here and we specifically open directories. + // + // At the root invocation of this function this will correctly handle + // symlinks passed to the top-level `remove_dir_all`. At the recursive + // level this will double-check that after the `readdir` call deduced this + // was a directory it's still a directory by the time we open it up. + // + // If the opened file was actually a symlink then the symlink is deleted, + // not the directory recursively. + let mut opts = OpenOptions::new(); + opts.lookup_flags(0); + opts.directory(true); + opts.read(true); + let fd = open_at(parent, path, &opts)?; + if fd.file_attr()?.file_type().is_symlink() { + return parent.unlink_file(osstr2str(path.as_ref())?); + } + + // this "root" is only used by `DirEntry::path` which we don't use below so + // it's ok for this to be a bogus value + let dummy_root = PathBuf::new(); + + // Iterate over all the entries in this directory, and travel recursively if + // necessary + for entry in ReadDir::new(fd, dummy_root) { + let entry = entry?; + let path = crate::str::from_utf8(&entry.name).map_err(|_| { + io::Error::new_const(io::ErrorKind::Uncategorized, &"invalid utf-8 file name found") + })?; + + if entry.file_type()?.is_dir() { + remove_dir_all_recursive(&entry.inner.dir.fd, path.as_ref())?; + } else { + entry.inner.dir.fd.unlink_file(path)?; + } + } + + // Once all this directory's contents are deleted it should be safe to + // delete the directory tiself. + parent.remove_directory(osstr2str(path.as_ref())?) +} From 32080ad6d0ecdc50c22d214cb64c07874e5ff3c8 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 19 Jan 2022 13:14:24 +0100 Subject: [PATCH 4/5] Update std::fs::remove_dir_all documentation --- library/std/src/fs.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index a00b5e1232369..c0b0d2389894e 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -2042,13 +2042,17 @@ pub fn remove_dir>(path: P) -> io::Result<()> { /// /// # Platform-specific behavior /// -/// This function currently corresponds to `opendir`, `lstat`, `rm` and `rmdir` functions on Unix -/// and the `FindFirstFile`, `GetFileAttributesEx`, `DeleteFile`, and `RemoveDirectory` functions -/// on Windows. -/// Note that, this [may change in the future][changes]. +/// This function currently corresponds to `openat`, `fdopendir`, `unlinkat` and `lstat` functions +/// on Unix (except for macOS before version 10.10 and REDOX) and the `CreateFileW`, +/// `GetFileInformationByHandleEx`, `SetFileInformationByHandle`, and `NtOpenFile` functions on +/// Windows. Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior /// +/// On macOS before version 10.10 and REDOX this function is not protected against time-of-check to +/// time-of-use (TOCTOU) race conditions, and should not be used in security-sensitive code on +/// those platforms. All other platforms are protected. +/// /// # Errors /// /// See [`fs::remove_file`] and [`fs::remove_dir`]. From 0a6c9adc4a09d6c34437a57e83adf78b2ad1e7db Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 20 Jan 2022 12:43:54 +0100 Subject: [PATCH 5/5] Fix compilation for a few tier 2 targets --- library/std/src/sys/unix/fs.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 16460bee66ff2..f8deda93fe2a7 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -670,12 +670,7 @@ impl DirEntry { fn name_cstr(&self) -> &CStr { unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } } - #[cfg(any( - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox" - ))] + #[cfg(any(target_os = "solaris", target_os = "illumos", target_os = "fuchsia"))] fn name_cstr(&self) -> &CStr { &self.name } @@ -1631,7 +1626,8 @@ mod remove_dir_impl { target_os = "solaris", target_os = "illumos", target_os = "haiku", - target_os = "vxworks" + target_os = "vxworks", + target_os = "fuchsia" ))] fn is_dir(_ent: &DirEntry) -> Option { None @@ -1641,7 +1637,8 @@ mod remove_dir_impl { target_os = "solaris", target_os = "illumos", target_os = "haiku", - target_os = "vxworks" + target_os = "vxworks", + target_os = "fuchsia" )))] fn is_dir(ent: &DirEntry) -> Option { match ent.entry.d_type {