diff --git a/cap-primitives/src/fs/dir_entry.rs b/cap-primitives/src/fs/dir_entry.rs index 1ed82752..04ee5f4e 100644 --- a/cap-primitives/src/fs/dir_entry.rs +++ b/cap-primitives/src/fs/dir_entry.rs @@ -1,4 +1,6 @@ -use crate::fs::{dir_options, DirEntryInner, FileType, Metadata, OpenOptions, ReadDir}; +use crate::fs::{ + dir_options, DirEntryInner, FileType, FollowSymlinks, Metadata, OpenOptions, ReadDir, +}; #[cfg(not(windows))] use rustix::fs::DirEntryExt; use std::ffi::OsString; @@ -57,7 +59,7 @@ impl DirEntry { /// Returns an iterator over the entries within the subdirectory. #[inline] pub fn read_dir(&self) -> io::Result { - self.inner.read_dir() + self.inner.read_dir(FollowSymlinks::Yes) } /// Returns the metadata for the file that this entry points at. diff --git a/cap-primitives/src/fs/file_path_by_searching.rs b/cap-primitives/src/fs/file_path_by_searching.rs index 1c96f6bd..a357f53a 100644 --- a/cap-primitives/src/fs/file_path_by_searching.rs +++ b/cap-primitives/src/fs/file_path_by_searching.rs @@ -1,4 +1,6 @@ -use crate::fs::{is_root_dir, open_dir_unchecked, read_dir_unchecked, MaybeOwnedFile, Metadata}; +use crate::fs::{ + is_root_dir, open_dir_unchecked, read_dir_unchecked, FollowSymlinks, MaybeOwnedFile, Metadata, +}; use std::fs; use std::path::{Component, PathBuf}; @@ -13,7 +15,8 @@ pub(crate) fn file_path_by_searching(file: &fs::File) -> Option { // Iterate with `..` until we reach the root directory. 'next_component: loop { // Open `..`. - let mut iter = read_dir_unchecked(&base, Component::ParentDir.as_ref()).ok()?; + let mut iter = + read_dir_unchecked(&base, Component::ParentDir.as_ref(), FollowSymlinks::No).ok()?; let metadata = Metadata::from_file(&*base).ok()?; // Search the children until we find one with matching metadata, and diff --git a/cap-primitives/src/fs/mod.rs b/cap-primitives/src/fs/mod.rs index 70853687..748c9f8a 100644 --- a/cap-primitives/src/fs/mod.rs +++ b/cap-primitives/src/fs/mod.rs @@ -54,7 +54,8 @@ pub(crate) use super::rustix::fs::*; #[cfg(windows)] pub(crate) use super::windows::fs::*; -pub(crate) use read_dir::read_dir_unchecked; +#[cfg(not(windows))] +pub(crate) use read_dir::{read_dir_nofollow, read_dir_unchecked}; pub use canonicalize::canonicalize; pub use copy::copy; diff --git a/cap-primitives/src/fs/open_dir.rs b/cap-primitives/src/fs/open_dir.rs index 7ac86b5c..ec6ab4c3 100644 --- a/cap-primitives/src/fs/open_dir.rs +++ b/cap-primitives/src/fs/open_dir.rs @@ -18,9 +18,14 @@ pub fn open_dir(start: &fs::File, path: &Path) -> io::Result { /// Like `open_dir`, but additionally request the ability to read the directory /// entries. +#[cfg(not(windows))] #[inline] -pub fn open_dir_for_reading(start: &fs::File, path: &Path) -> io::Result { - open(start, path, &readdir_options()) +pub(crate) fn open_dir_for_reading( + start: &fs::File, + path: &Path, + follow: FollowSymlinks, +) -> io::Result { + open(start, path, readdir_options().follow(follow)) } /// Similar to `open_dir`, but fails if the path names a symlink. @@ -43,8 +48,9 @@ pub(crate) fn open_dir_unchecked(start: &fs::File, path: &Path) -> io::Result io::Result { - open_unchecked(start, path, &readdir_options()).map_err(Into::into) + open_unchecked(start, path, readdir_options().follow(follow)).map_err(Into::into) } /// Open a directory named by a bare path, using the host process' ambient diff --git a/cap-primitives/src/fs/read_dir.rs b/cap-primitives/src/fs/read_dir.rs index c3b9125b..2ec908cf 100644 --- a/cap-primitives/src/fs/read_dir.rs +++ b/cap-primitives/src/fs/read_dir.rs @@ -1,4 +1,4 @@ -use crate::fs::{DirEntry, ReadDirInner}; +use crate::fs::{DirEntry, FollowSymlinks, ReadDirInner}; use std::path::Path; use std::{fmt, fs, io}; @@ -8,7 +8,16 @@ use std::{fmt, fs, io}; #[inline] pub fn read_dir(start: &fs::File, path: &Path) -> io::Result { Ok(ReadDir { - inner: ReadDirInner::new(start, path)?, + inner: ReadDirInner::new(start, path, FollowSymlinks::Yes)?, + }) +} + +/// Like `read_dir`, but fails if `path` names a symlink. +#[inline] +#[cfg(not(windows))] +pub(crate) fn read_dir_nofollow(start: &fs::File, path: &Path) -> io::Result { + Ok(ReadDir { + inner: ReadDirInner::new(start, path, FollowSymlinks::No)?, }) } @@ -23,9 +32,14 @@ pub fn read_base_dir(start: &fs::File) -> io::Result { /// Like `read_dir`, but doesn't perform sandboxing. #[inline] -pub(crate) fn read_dir_unchecked(start: &fs::File, path: &Path) -> io::Result { +#[cfg(not(windows))] +pub(crate) fn read_dir_unchecked( + start: &fs::File, + path: &Path, + follow: FollowSymlinks, +) -> io::Result { Ok(ReadDir { - inner: ReadDirInner::new_unchecked(start, path)?, + inner: ReadDirInner::new_unchecked(start, path, follow)?, }) } diff --git a/cap-primitives/src/rustix/fs/dir_entry_inner.rs b/cap-primitives/src/rustix/fs/dir_entry_inner.rs index a6c75ce5..3698d03e 100644 --- a/cap-primitives/src/rustix/fs/dir_entry_inner.rs +++ b/cap-primitives/src/rustix/fs/dir_entry_inner.rs @@ -1,4 +1,6 @@ -use crate::fs::{FileType, FileTypeExt, Metadata, OpenOptions, ReadDir, ReadDirInner}; +use crate::fs::{ + FileType, FileTypeExt, FollowSymlinks, Metadata, OpenOptions, ReadDir, ReadDirInner, +}; use rustix::fs::DirEntry; use std::ffi::{OsStr, OsString}; #[cfg(unix)] @@ -29,8 +31,8 @@ impl DirEntryInner { } #[inline] - pub(crate) fn read_dir(&self) -> io::Result { - self.read_dir.read_dir(self.file_name_bytes()) + pub(crate) fn read_dir(&self, follow: FollowSymlinks) -> io::Result { + self.read_dir.read_dir(self.file_name_bytes(), follow) } #[inline] diff --git a/cap-primitives/src/rustix/fs/read_dir_inner.rs b/cap-primitives/src/rustix/fs/read_dir_inner.rs index 9e34860f..41f555cf 100644 --- a/cap-primitives/src/rustix/fs/read_dir_inner.rs +++ b/cap-primitives/src/rustix/fs/read_dir_inner.rs @@ -22,8 +22,8 @@ pub(crate) struct ReadDirInner { } impl ReadDirInner { - pub(crate) fn new(start: &fs::File, path: &Path) -> io::Result { - let dir = Dir::from(open_dir_for_reading(start, path)?)?; + pub(crate) fn new(start: &fs::File, path: &Path, follow: FollowSymlinks) -> io::Result { + let dir = Dir::from(open_dir_for_reading(start, path, follow)?)?; Ok(Self { raw_fd: dir.as_fd().as_raw_fd(), rustix: Arc::new(Mutex::new(dir)), @@ -38,6 +38,7 @@ impl ReadDirInner { let dir = Dir::from(open_dir_for_reading_unchecked( start, Component::CurDir.as_ref(), + FollowSymlinks::No, )?)?; Ok(Self { raw_fd: dir.as_fd().as_raw_fd(), @@ -45,8 +46,12 @@ impl ReadDirInner { }) } - pub(crate) fn new_unchecked(start: &fs::File, path: &Path) -> io::Result { - let dir = open_dir_for_reading_unchecked(start, path)?; + pub(crate) fn new_unchecked( + start: &fs::File, + path: &Path, + follow: FollowSymlinks, + ) -> io::Result { + let dir = open_dir_for_reading_unchecked(start, path, follow)?; Ok(Self { raw_fd: dir.as_fd().as_raw_fd(), rustix: Arc::new(Mutex::new(Dir::from(dir)?)), @@ -73,8 +78,12 @@ impl ReadDirInner { Metadata::from_file(&self.as_file_view()) } - pub(super) fn read_dir(&self, file_name: &OsStr) -> io::Result { - read_dir_unchecked(&self.as_file_view(), file_name.as_ref()) + pub(super) fn read_dir( + &self, + file_name: &OsStr, + follow: FollowSymlinks, + ) -> io::Result { + read_dir_unchecked(&self.as_file_view(), file_name.as_ref(), follow) } #[allow(unsafe_code)] diff --git a/cap-primitives/src/rustix/fs/remove_dir_all_impl.rs b/cap-primitives/src/rustix/fs/remove_dir_all_impl.rs index 52d79806..cf2ed1db 100644 --- a/cap-primitives/src/rustix/fs/remove_dir_all_impl.rs +++ b/cap-primitives/src/rustix/fs/remove_dir_all_impl.rs @@ -1,6 +1,6 @@ use crate::fs::{ - read_dir, read_dir_unchecked, remove_dir, remove_file, remove_open_dir, stat, FollowSymlinks, - ReadDir, + read_dir_nofollow, read_dir_unchecked, remove_dir, remove_file, remove_open_dir, stat, + FollowSymlinks, ReadDir, }; use std::path::{Component, Path}; use std::{fs, io}; @@ -13,13 +13,17 @@ pub(crate) fn remove_dir_all_impl(start: &fs::File, path: &Path) -> io::Result<( if filetype.is_symlink() { remove_file(start, path) } else { - remove_dir_all_recursive(read_dir(start, path)?)?; + remove_dir_all_recursive(read_dir_nofollow(start, path)?)?; remove_dir(start, path) } } pub(crate) fn remove_open_dir_all_impl(dir: fs::File) -> io::Result<()> { - remove_dir_all_recursive(read_dir_unchecked(&dir, Component::CurDir.as_ref())?)?; + remove_dir_all_recursive(read_dir_unchecked( + &dir, + Component::CurDir.as_ref(), + FollowSymlinks::No, + )?)?; remove_open_dir(dir) } @@ -27,7 +31,7 @@ fn remove_dir_all_recursive(children: ReadDir) -> io::Result<()> { for child in children { let child = child?; if child.file_type()?.is_dir() { - remove_dir_all_recursive(child.read_dir()?)?; + remove_dir_all_recursive(child.inner.read_dir(FollowSymlinks::No)?)?; child.remove_dir()?; } else { child.remove_file()?; diff --git a/cap-primitives/src/rustix/fs/remove_open_dir_by_searching.rs b/cap-primitives/src/rustix/fs/remove_open_dir_by_searching.rs index 45c819e0..7f1d5592 100644 --- a/cap-primitives/src/rustix/fs/remove_open_dir_by_searching.rs +++ b/cap-primitives/src/rustix/fs/remove_open_dir_by_searching.rs @@ -1,4 +1,4 @@ -use crate::fs::{errors, is_root_dir, read_dir_unchecked, Metadata}; +use crate::fs::{errors, is_root_dir, read_dir_unchecked, FollowSymlinks, Metadata}; use std::path::Component; use std::{fs, io}; @@ -7,7 +7,7 @@ use std::{fs, io}; /// available. pub(crate) fn remove_open_dir_by_searching(dir: fs::File) -> io::Result<()> { let metadata = Metadata::from_file(&dir)?; - let mut iter = read_dir_unchecked(&dir, Component::ParentDir.as_ref())?; + let mut iter = read_dir_unchecked(&dir, Component::ParentDir.as_ref(), FollowSymlinks::No)?; while let Some(child) = iter.next() { let child = child?; if child.is_same_file(&metadata)? { diff --git a/cap-primitives/src/windows/fs/dir_entry_inner.rs b/cap-primitives/src/windows/fs/dir_entry_inner.rs index a00f3673..6da7a78c 100644 --- a/cap-primitives/src/windows/fs/dir_entry_inner.rs +++ b/cap-primitives/src/windows/fs/dir_entry_inner.rs @@ -68,7 +68,12 @@ impl DirEntryInner { } #[inline] - pub(crate) fn read_dir(&self) -> io::Result { + pub(crate) fn read_dir(&self, follow: FollowSymlinks) -> io::Result { + assert_eq!( + follow, + FollowSymlinks::Yes, + "`read_dir` without following symlinks is not implemented yet" + ); let std = fs::read_dir(self.std.path())?; let inner = ReadDirInner::from_std(std); Ok(ReadDir { inner }) diff --git a/cap-primitives/src/windows/fs/read_dir_inner.rs b/cap-primitives/src/windows/fs/read_dir_inner.rs index a4b9cb39..92c7ef8b 100644 --- a/cap-primitives/src/windows/fs/read_dir_inner.rs +++ b/cap-primitives/src/windows/fs/read_dir_inner.rs @@ -1,5 +1,5 @@ use super::get_path::concatenate_or_return_absolute; -use crate::fs::{open_dir, DirEntryInner}; +use crate::fs::{open_dir, DirEntryInner, FollowSymlinks}; use std::path::{Component, Path}; use std::{fmt, fs, io}; @@ -8,7 +8,12 @@ pub(crate) struct ReadDirInner { } impl ReadDirInner { - pub(crate) fn new(start: &fs::File, path: &Path) -> io::Result { + pub(crate) fn new(start: &fs::File, path: &Path, follow: FollowSymlinks) -> io::Result { + assert_eq!( + follow, + FollowSymlinks::Yes, + "`read_dir` without following symlinks is not implemented yet" + ); let dir = open_dir(start, path)?; Self::new_unchecked(&dir, Component::CurDir.as_ref()) } diff --git a/cap-primitives/src/windows/fs/remove_dir_all_impl.rs b/cap-primitives/src/windows/fs/remove_dir_all_impl.rs index ffd1fd11..2147ceb5 100644 --- a/cap-primitives/src/windows/fs/remove_dir_all_impl.rs +++ b/cap-primitives/src/windows/fs/remove_dir_all_impl.rs @@ -1,74 +1,30 @@ -use crate::fs::{ - read_dir_unchecked, remove_dir, remove_file, remove_open_dir, stat, FollowSymlinks, -}; +use super::get_path::get_path; +use crate::fs::{open_dir, open_dir_nofollow, remove_dir, stat, FollowSymlinks}; #[cfg(windows_file_type_ext)] use std::os::windows::fs::FileTypeExt; -use std::path::{Component, Path}; +use std::path::Path; use std::{fs, io}; pub(crate) fn remove_dir_all_impl(start: &fs::File, path: &Path) -> io::Result<()> { - // Code derived from `remove_dir_all` in Rust's - // library/std/src/sys/windows/fs.rs at revision - // 108e90ca78f052c0c1c49c42a22c85620be19712. - let filetype = stat(start, path, FollowSymlinks::No)?.file_type(); - if filetype.is_symlink() { - // On Windows symlinks to files and directories are removed - // differently. `remove_dir` only deletes dir symlinks and junctions, - // not file symlinks. + // Open the directory, following symlinks, to make sure it is a directory. + let file = open_dir(start, path)?; + // Test whether the path is a symlink. + let md = stat(start, path, FollowSymlinks::No)?; + drop(file); + if md.is_symlink() { + // If so, just remove the link. remove_dir(start, path) } else { - remove_dir_all_recursive(start, path)?; - remove_dir(start, path) + // Otherwise, remove the tree. + let dir = open_dir_nofollow(start, path)?; + remove_open_dir_all_impl(dir) } } pub(crate) fn remove_open_dir_all_impl(dir: fs::File) -> io::Result<()> { - remove_dir_all_recursive(&dir, Component::CurDir.as_ref())?; - remove_open_dir(dir) -} - -#[cfg(windows_file_type_ext)] -fn remove_dir_all_recursive(start: &fs::File, path: &Path) -> io::Result<()> { - // Code derived from `remove_dir_all_recursive` in Rust's - // library/std/src/sys/windows/fs.rs at revision - // 108e90ca78f052c0c1c49c42a22c85620be19712. - for child in read_dir_unchecked(start, path)? { - let child = child?; - let child_type = child.file_type()?; - if child_type.is_dir() { - let path = path.join(child.file_name()); - remove_dir_all_recursive(start, &path)?; - remove_dir(start, &path)?; - } else if child_type.is_symlink_dir() { - remove_dir(start, &path.join(child.file_name()))?; - } else { - remove_file(start, &path.join(child.file_name()))?; - } - } - Ok(()) -} - -#[cfg(not(windows_file_type_ext))] -fn remove_dir_all_recursive(start: &fs::File, path: &Path) -> io::Result<()> { - for child in read_dir_unchecked(start, path)? { - let child = child?; - let child_type = child.file_type()?; - if child_type.is_dir() { - let path = path.join(child.file_name()); - remove_dir_all_recursive(start, &path)?; - remove_dir(start, &path)?; - } else { - match remove_dir(start, &path.join(child.file_name())) { - Ok(()) => (), - Err(e) => { - if e.raw_os_error() == Some(winapi::shared::winerror::ERROR_DIRECTORY as i32) { - remove_file(start, &path.join(child.file_name()))?; - } else { - return Err(e); - } - } - } - } - } - Ok(()) + // Close the directory so that we can delete it. This is racy; see the + // comments in `remove_open_dir_impl` for details. + let path = get_path(&dir)?; + drop(dir); + fs::remove_dir_all(&path) } diff --git a/cap-tempfile/src/lib.rs b/cap-tempfile/src/lib.rs index a00d6f75..17cae23c 100644 --- a/cap-tempfile/src/lib.rs +++ b/cap-tempfile/src/lib.rs @@ -218,10 +218,11 @@ fn close_outer() { let t = tempdir(ambient_authority()).unwrap(); let _s = tempdir_in(&t).unwrap(); #[cfg(windows)] - assert_eq!( - t.close().unwrap_err().raw_os_error(), - Some(winapi::shared::winerror::ERROR_SHARING_VIOLATION as i32) - ); + assert!(matches!( + t.close().unwrap_err().raw_os_error().map(|err| err as _), + Some(winapi::shared::winerror::ERROR_SHARING_VIOLATION) + | Some(winapi::shared::winerror::ERROR_DIR_NOT_EMPTY) + )); #[cfg(not(windows))] t.close().unwrap(); } diff --git a/cap-tempfile/src/utf8.rs b/cap-tempfile/src/utf8.rs index 37782410..2eec5e11 100644 --- a/cap-tempfile/src/utf8.rs +++ b/cap-tempfile/src/utf8.rs @@ -214,10 +214,11 @@ fn close_outer() { let t = tempdir(ambient_authority()).unwrap(); let _s = tempdir_in(&t).unwrap(); #[cfg(windows)] - assert_eq!( - t.close().unwrap_err().raw_os_error(), - Some(winapi::shared::winerror::ERROR_SHARING_VIOLATION as i32) - ); + assert!(matches!( + t.close().unwrap_err().raw_os_error().map(|err| err as _), + Some(winapi::shared::winerror::ERROR_SHARING_VIOLATION) + | Some(winapi::shared::winerror::ERROR_DIR_NOT_EMPTY) + )); #[cfg(not(windows))] t.close().unwrap(); } diff --git a/tests/fs.rs b/tests/fs.rs index d59ae792..b51d5183 100644 --- a/tests/fs.rs +++ b/tests/fs.rs @@ -19,8 +19,10 @@ use libc::{c_char, c_int}; use std::io::{self, ErrorKind, SeekFrom}; use std::path::{Path, PathBuf}; use std::str; +use std::sync::Arc; #[cfg(not(racy_asserts))] // racy asserts are racy use std::thread; +use std::time::{Duration, Instant}; use sys_common::io::{tmpdir, TempDir}; use sys_common::symlink_junction; @@ -620,6 +622,21 @@ fn recursive_rmdir_of_symlink() { assert!(tmpdir.exists(canary)); } +#[test] +fn recursive_rmdir_of_file_fails() { + // test we do not delete a directly specified file. + let tmpdir = tmpdir(); + let canary = "do_not_delete"; + check!(check!(tmpdir.create(canary)).write(b"foo")); + let result = tmpdir.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!(tmpdir.exists(canary)); +} + #[test] // only Windows makes a distinction between file and directory symlinks. #[cfg(windows)] @@ -639,6 +656,60 @@ 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 = "victim_del"; + let victim_del_path_clone = victim_del_path.clone(); + + // setup dest + let attack_dest_dir = "attack_dest"; + let attack_dest_dir = Path::new(attack_dest_dir); + tmpdir.create_dir(attack_dest_dir).unwrap(); + let attack_dest_file = "attack_dest/attack_file"; + tmpdir.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` + let tmpdir_clone = tmpdir.try_clone().unwrap(); + let _t = thread::spawn(move || { + while drop_canary_weak.upgrade().is_some() { + let _ = tmpdir_clone.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 !tmpdir.exists(attack_dest_file) { + panic!( + "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.", + Instant::now().duration_since(start_time) + ); + } + let _ = tmpdir.create_dir(victim_del_path); + let _ = tmpdir.remove_dir(victim_del_path); + let _ = symlink_dir(attack_dest_dir, &tmpdir, victim_del_path); + } +} + #[test] fn unicode_path_is_dir() { let tmpdir = tmpdir(); diff --git a/tests/fs_utf8.rs b/tests/fs_utf8.rs index 9ab0c438..911afb9d 100644 --- a/tests/fs_utf8.rs +++ b/tests/fs_utf8.rs @@ -20,8 +20,10 @@ use cap_std::fs_utf8::{self as fs, Dir, OpenOptions}; use libc::{c_char, c_int}; use std::io::{self, ErrorKind, SeekFrom}; use std::str; +use std::sync::Arc; #[cfg(not(racy_asserts))] // racy asserts are racy use std::thread; +use std::time::{Duration, Instant}; use sys_common::io::{tmpdir_utf8 as tmpdir, TempDirUtf8 as TempDir}; use sys_common::symlink_junction_utf8 as symlink_junction; @@ -621,6 +623,21 @@ fn recursive_rmdir_of_symlink() { assert!(tmpdir.exists(canary)); } +#[test] +fn recursive_rmdir_of_file_fails() { + // test we do not delete a directly specified file. + let tmpdir = tmpdir(); + let canary = "do_not_delete"; + check!(check!(tmpdir.create(canary)).write(b"foo")); + let result = tmpdir.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!(tmpdir.exists(canary)); +} + #[test] // only Windows makes a distinction between file and directory symlinks. #[cfg(windows)] @@ -640,6 +657,60 @@ 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 = "victim_del"; + let victim_del_path_clone = victim_del_path.clone(); + + // setup dest + let attack_dest_dir = "attack_dest"; + let attack_dest_dir = Path::new(attack_dest_dir); + tmpdir.create_dir(attack_dest_dir).unwrap(); + let attack_dest_file = "attack_dest/attack_file"; + tmpdir.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` + let tmpdir_clone = tmpdir.try_clone().unwrap(); + let _t = thread::spawn(move || { + while drop_canary_weak.upgrade().is_some() { + let _ = tmpdir_clone.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 !tmpdir.exists(attack_dest_file) { + panic!( + "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.", + Instant::now().duration_since(start_time) + ); + } + let _ = tmpdir.create_dir(victim_del_path); + let _ = tmpdir.remove_dir(victim_del_path); + let _ = symlink_dir(attack_dest_dir, &tmpdir, victim_del_path); + } +} + #[test] fn unicode_path_is_dir() { let tmpdir = tmpdir(); diff --git a/tests/sys/unix/weak.rs b/tests/sys/unix/weak.rs index 1a5aa9f0..030e818a 100644 --- a/tests/sys/unix/weak.rs +++ b/tests/sys/unix/weak.rs @@ -84,8 +84,11 @@ impl ExternWeak { #[macro_export] macro_rules! 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: $crate::sys::weak::DlsymWeak $ret> = - $crate::sys::weak::DlsymWeak::new(concat!(stringify!($name), '\0')); + $crate::sys::weak::DlsymWeak::new(concat!($sym, '\0')); let $name = &DLSYM; ) }