diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index 817fc1e6b28..2f26340acdc 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -116,6 +116,7 @@ noxfer ofile oflag oflags +openat pdeathsig peekable performant diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 127464dc1af..d77378da453 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -19,7 +19,7 @@ use uucore::mode; use uucore::perms::{TraverseSymlinks, configure_symlink_and_recursion}; #[cfg(all(unix, not(target_os = "redox")))] -use uucore::safe_traversal::DirFd; +use uucore::safe_traversal::{DirFd, SymlinkBehavior}; use uucore::{format_usage, show, show_error}; use uucore::translate; @@ -473,7 +473,7 @@ impl Chmoder { // If the path is a directory (or we should follow symlinks), recurse into it using safe traversal if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() { - match DirFd::open(file_path) { + match DirFd::open(file_path, SymlinkBehavior::Follow) { Ok(dir_fd) => { r = self.safe_traverse_dir(&dir_fd, file_path).and(r); } @@ -502,7 +502,7 @@ impl Chmoder { for entry_name in entries { let entry_path = dir_path.join(&entry_name); - let dir_meta = dir_fd.metadata_at(&entry_name, should_follow_symlink); + let dir_meta = dir_fd.metadata_at(&entry_name, should_follow_symlink.into()); let Ok(meta) = dir_meta else { // Handle permission denied with proper file path context let e = dir_meta.unwrap_err(); @@ -527,7 +527,7 @@ impl Chmoder { // Recurse into subdirectories using the existing directory fd if meta.is_dir() { - match dir_fd.open_subdir(&entry_name) { + match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) { Ok(child_dir_fd) => { r = self.safe_traverse_dir(&child_dir_fd, &entry_path).and(r); } @@ -591,7 +591,7 @@ impl Chmoder { // Use safe traversal to change the mode let follow_symlinks = self.dereference; - if let Err(_e) = dir_fd.chmod_at(entry_name, new_mode, follow_symlinks) { + if let Err(_e) = dir_fd.chmod_at(entry_name, new_mode, follow_symlinks.into()) { if self.verbose { println!( "failed to change mode of {} to {new_mode:o}", diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 46c51440d31..31eadfa52be 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -26,7 +26,7 @@ use uucore::error::{FromIo, UError, UResult, USimpleError, set_exit_code}; use uucore::fsext::{MetadataTimeField, metadata_get_time}; use uucore::line_ending::LineEnding; #[cfg(all(unix, not(target_os = "redox")))] -use uucore::safe_traversal::DirFd; +use uucore::safe_traversal::{DirFd, SymlinkBehavior}; use uucore::translate; use uucore::parser::parse_glob; @@ -313,7 +313,7 @@ fn safe_du( let mut my_stat = if let Some(parent_fd) = parent_fd { // We have a parent fd, this is a subdirectory - use openat let dir_name = path.file_name().unwrap_or(path.as_os_str()); - match parent_fd.metadata_at(dir_name, false) { + match parent_fd.metadata_at(dir_name, SymlinkBehavior::NoFollow) { Ok(safe_metadata) => { // Create Stat from safe metadata let file_info = safe_metadata.file_info(); @@ -368,7 +368,7 @@ fn safe_du( Ok(s) => s, Err(_e) => { // Try using our new DirFd method for the root directory - match DirFd::open(path) { + match DirFd::open(path, SymlinkBehavior::Follow) { Ok(dir_fd) => match Stat::new_from_dirfd(&dir_fd, path) { Ok(s) => s, Err(e) => { @@ -406,8 +406,11 @@ fn safe_du( // Open the directory using DirFd let open_result = match parent_fd { - Some(parent) => parent.open_subdir(path.file_name().unwrap_or(path.as_os_str())), - None => DirFd::open(path), + Some(parent) => parent.open_subdir( + path.file_name().unwrap_or(path.as_os_str()), + SymlinkBehavior::Follow, + ), + None => DirFd::open(path, SymlinkBehavior::Follow), }; let dir_fd = match open_result { @@ -435,7 +438,7 @@ fn safe_du( let entry_path = path.join(&entry_name); // First get the lstat (without following symlinks) to check if it's a symlink - let lstat = match dir_fd.stat_at(&entry_name, false) { + let lstat = match dir_fd.stat_at(&entry_name, SymlinkBehavior::NoFollow) { Ok(stat) => stat, Err(e) => { print_tx.send(Err(e.map_err_context( diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 84eefdcc870..b46c4a40a30 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -28,6 +28,8 @@ use uucore::error::{FromIo, UError, UResult, UUsageError}; use uucore::fs::dir_strip_dot_for_creation; use uucore::perms::{Verbosity, VerbosityLevel, wrap_chown}; use uucore::process::{getegid, geteuid}; +#[cfg(unix)] +use uucore::safe_traversal::{DirFd, SymlinkBehavior, create_dir_all_safe}; #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] use uucore::selinux::{ SeLinuxError, contexts_differ, get_selinux_security_context, is_selinux_enabled, @@ -490,7 +492,7 @@ fn directory(paths: &[OsString], b: &Behavior) -> UResult<()> { } // Set SELinux context for all created directories if needed - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] if should_set_selinux_context(b) { let context = get_context_for_selinux(b); set_selinux_context_for_directories_install(path_to_create.as_path(), context); @@ -515,7 +517,7 @@ fn directory(paths: &[OsString], b: &Behavior) -> UResult<()> { show_if_err!(chown_optional_user_group(path, b)); // Set SELinux context for directory if needed - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] if b.default_context { show_if_err!(set_selinux_default_context(path)); } else if b.context.is_some() { @@ -594,6 +596,11 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { let sources = &paths.iter().map(PathBuf::from).collect::>(); + #[cfg(unix)] + let mut target_parent_fd: Option = None; + #[cfg(unix)] + let mut target_filename: Option = None; + if b.create_leading { // if -t is used in combination with -D, create whole target because it does not include filename let to_create: Option<&Path> = if b.target_dir.is_some() { @@ -605,8 +612,13 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { None }; + // If -t is used, check if target exists as a file before trying to create directories + if b.target_dir.is_some() && target.exists() && !target.is_dir() { + return Err(InstallError::NotADirectory(target.clone()).into()); + } + if let Some(to_create) = to_create { - // if the path ends in /, remove it + let to_create_original = to_create; let to_create_owned; let to_create = match uucore::os_str_as_bytes(to_create.as_os_str()) { Ok(path_bytes) if path_bytes.ends_with(b"/") => { @@ -621,7 +633,30 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { _ => to_create, }; - if !to_create.exists() { + let dir_exists = if to_create.exists() { + fs::symlink_metadata(to_create) + .map(|m| m.is_dir() && !m.file_type().is_symlink()) + .unwrap_or(false) + } else { + false + }; + + if dir_exists { + #[cfg(unix)] + { + if b.target_dir.is_none() + && sources.len() == 1 + && !is_potential_directory_path(&target) + { + if let Ok(dir_fd) = DirFd::open(to_create, SymlinkBehavior::NoFollow) { + if let Some(filename) = target.file_name() { + target_parent_fd = Some(dir_fd); + target_filename = Some(filename.to_os_string()); + } + } + } + } + } else { if b.verbose { let mut result = PathBuf::new(); // When creating directories with -Dv, show directory creations step by step @@ -638,23 +673,63 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { } } - if let Err(e) = fs::create_dir_all(to_create) { - return Err(InstallError::CreateDirFailed(to_create.to_path_buf(), e).into()); + #[cfg(unix)] + { + // Use DEFAULT_MODE (0o755) for created directories - this matches GNU install + // behavior. The actual mode will be modified by umask at the kernel level. + match create_dir_all_safe(to_create, DEFAULT_MODE) { + Ok(dir_fd) => { + if b.target_dir.is_none() + && sources.len() == 1 + && !is_potential_directory_path(&target) + { + if let Some(filename) = target.file_name() { + target_parent_fd = Some(dir_fd); + target_filename = Some(filename.to_os_string()); + } + } + + // Set SELinux context for all created directories if needed + #[cfg(all(feature = "selinux", target_os = "linux"))] + if should_set_selinux_context(b) { + let context = get_context_for_selinux(b); + set_selinux_context_for_directories_install(to_create, context); + } + } + Err(e) => { + if e.kind() == std::io::ErrorKind::AlreadyExists + && to_create.exists() + && !to_create.is_dir() + { + return Err(InstallError::NotADirectory( + to_create_original.to_path_buf(), + ) + .into()); + } + return Err(InstallError::CreateDirFailed( + to_create_original.to_path_buf(), + e, + ) + .into()); + } + } } - // Set SELinux context for all created directories if needed - #[cfg(feature = "selinux")] - if should_set_selinux_context(b) { - let context = get_context_for_selinux(b); - set_selinux_context_for_directories_install(to_create, context); - } - } - } - if b.target_dir.is_some() { - let p = to_create.unwrap(); + #[cfg(not(unix))] + { + if let Err(e) = fs::create_dir_all(to_create) { + return Err( + InstallError::CreateDirFailed(to_create.to_path_buf(), e).into() + ); + } - if !p.exists() || !p.is_dir() { - return Err(InstallError::NotADirectory(p.to_path_buf()).into()); + // Set SELinux context for all created directories if needed + #[cfg(all(feature = "selinux", target_os = "linux"))] + if should_set_selinux_context(b) { + let context = get_context_for_selinux(b); + set_selinux_context_for_directories_install(to_create, context); + } + } } } } @@ -679,7 +754,33 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { } if target.is_file() || is_new_file_path(&target) { - copy(source, &target, b) + #[cfg(unix)] + if let (Some(ref parent_fd), Some(ref filename)) = (target_parent_fd, target_filename) { + if b.compare && !need_copy(source, &target, b) { + return Ok(()); + } + + let backup_path = perform_backup(&target, b)?; + + if let Err(e) = parent_fd.unlink_at(filename.as_os_str(), false) { + if e.kind() != std::io::ErrorKind::NotFound { + show_error!( + "{}", + translate!("install-error-failed-to-remove", "path" => target.quote(), "error" => format!("{e:?}")) + ); + } + } + + copy_file_safe(source, parent_fd, filename.as_os_str())?; + + finalize_installed_file(source, &target, b, backup_path) + } else { + copy(source, &target, b) + } + #[cfg(not(unix))] + { + copy(source, &target, b) + } } else { Err(InstallError::InvalidTarget(target).into()) } @@ -800,6 +901,38 @@ fn perform_backup(to: &Path, b: &Behavior) -> UResult> { } } +/// Copy a file using directory file descriptor for safe traversal. +/// +/// This is the fd-based counterpart to `copy_file`. It prevents symlink race +/// conditions by using `openat` to create the destination file relative to a +/// directory file descriptor, rather than using path-based operations. +/// +/// Note: This function and `copy_file` share similar logic but cannot easily +/// be consolidated because they use fundamentally different APIs: +/// - `copy_file_safe` uses fd-based `DirFd::open_file_at()` (openat syscall) +/// - `copy_file` uses path-based `OpenOptions::new().create_new().open()` +#[cfg(unix)] +fn copy_file_safe(from: &Path, to_parent_fd: &DirFd, to_filename: &std::ffi::OsStr) -> UResult<()> { + let from_meta = metadata(from)?; + + // Check if source and destination are the same file + if let Ok(to_stat) = to_parent_fd.stat_at(to_filename, SymlinkBehavior::Follow) { + // st_dev and st_ino types vary by platform (i32/u32 on macOS, u64 on Linux) + #[allow(clippy::unnecessary_cast)] + if from_meta.dev() == to_stat.st_dev as u64 && from_meta.ino() == to_stat.st_ino as u64 { + return Err( + InstallError::SameFile(from.to_path_buf(), PathBuf::from(to_filename)).into(), + ); + } + } + + let mut src = File::open(from)?; + let mut dst = to_parent_fd.open_file_at(to_filename)?; + copy_stream(&mut src, &mut dst)?; + + Ok(()) +} + /// Copy a file from one path to another. Handles the certain cases of special /// files (e.g character specials). /// @@ -828,9 +961,7 @@ fn copy_file(from: &Path, to: &Path) -> UResult<()> { .into()); } - // Remove existing file at destination to allow overwriting - // Note: create_new() below provides TOCTOU protection; if something - // appears at this path between the remove and create, it will fail safely + // Remove existing file (create_new below provides TOCTOU protection) if let Err(e) = fs::remove_file(to) { if e.kind() != std::io::ErrorKind::NotFound { show_error!( @@ -948,28 +1079,13 @@ fn preserve_timestamps(from: &Path, to: &Path) -> UResult<()> { Ok(()) } -/// Copy one file to a new location, changing metadata. -/// -/// Returns a Result type with the Err variant containing the error message. -/// -/// # Parameters -/// -/// _from_ must exist as a non-directory. -/// _to_ must be a non-existent file, whose parent directory exists. -/// -/// # Errors -/// -/// If the copy system call fails, we print a verbose error and return an empty error value. -/// -fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { - if b.compare && !need_copy(from, to, b) { - return Ok(()); - } - // Declare the path here as we may need it for the verbose output below. - let backup_path = perform_backup(to, b)?; - - copy_file(from, to)?; - +/// Apply post-copy operations: strip, ownership, permissions, timestamps, SELinux, and verbose output. +fn finalize_installed_file( + from: &Path, + to: &Path, + b: &Behavior, + backup_path: Option, +) -> UResult<()> { #[cfg(not(windows))] if b.strip { strip_file(to, b)?; @@ -981,7 +1097,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { preserve_timestamps(from, to)?; } - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] if !b.unprivileged { if b.preserve_context { uucore::selinux::preserve_security_context(from, to) @@ -1015,6 +1131,31 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { Ok(()) } +/// Copy one file to a new location, changing metadata. +/// +/// Returns a Result type with the Err variant containing the error message. +/// +/// # Parameters +/// +/// _from_ must exist as a non-directory. +/// _to_ must be a non-existent file, whose parent directory exists. +/// +/// # Errors +/// +/// If the copy system call fails, we print a verbose error and return an empty error value. +/// +fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { + if b.compare && !need_copy(from, to, b) { + return Ok(()); + } + // Declare the path here as we may need it for the verbose output below. + let backup_path = perform_backup(to, b)?; + + copy_file(from, to)?; + + finalize_installed_file(from, to, b, backup_path) +} + #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] fn get_context_for_selinux(b: &Behavior) -> Option<&String> { if b.default_context { @@ -1024,7 +1165,7 @@ fn get_context_for_selinux(b: &Behavior) -> Option<&String> { } } -#[cfg(feature = "selinux")] +#[cfg(all(feature = "selinux", target_os = "linux"))] fn should_set_selinux_context(b: &Behavior) -> bool { !b.unprivileged && (b.context.is_some() || b.default_context) } @@ -1119,7 +1260,7 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool { return true; } - #[cfg(feature = "selinux")] + #[cfg(all(feature = "selinux", target_os = "linux"))] if !b.unprivileged && b.preserve_context && contexts_differ(from, to) { return true; } diff --git a/src/uu/rm/src/platform/unix.rs b/src/uu/rm/src/platform/unix.rs index e890ab15823..687cffaf391 100644 --- a/src/uu/rm/src/platform/unix.rs +++ b/src/uu/rm/src/platform/unix.rs @@ -16,7 +16,7 @@ use std::path::Path; use uucore::display::Quotable; use uucore::error::FromIo; use uucore::prompt_yes; -use uucore::safe_traversal::DirFd; +use uucore::safe_traversal::{DirFd, SymlinkBehavior}; use uucore::show_error; use uucore::translate; @@ -120,7 +120,7 @@ pub fn safe_remove_file( let parent = path.parent().unwrap_or(Path::new(".")); let file_name = path.file_name()?; - let dir_fd = DirFd::open(parent).ok()?; + let dir_fd = DirFd::open(parent, SymlinkBehavior::Follow).ok()?; match dir_fd.unlink_at(file_name, false) { Ok(_) => { @@ -151,7 +151,7 @@ pub fn safe_remove_empty_dir( let parent = path.parent().unwrap_or(Path::new(".")); let dir_name = path.file_name()?; - let dir_fd = DirFd::open(parent).ok()?; + let dir_fd = DirFd::open(parent, SymlinkBehavior::Follow).ok()?; match dir_fd.unlink_at(dir_name, true) { Ok(_) => { @@ -290,7 +290,7 @@ pub fn safe_remove_dir_recursive( }; // Try to open the directory using DirFd for secure traversal - let dir_fd = match DirFd::open(path) { + let dir_fd = match DirFd::open(path, SymlinkBehavior::Follow) { Ok(fd) => fd, Err(e) => { // If we can't open the directory for safe traversal, @@ -368,7 +368,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt let entry_path = path.join(&entry_name); // Get metadata for the entry using fstatat - let entry_stat = match dir_fd.stat_at(&entry_name, false) { + let entry_stat = match dir_fd.stat_at(&entry_name, SymlinkBehavior::NoFollow) { Ok(stat) => stat, Err(e) => { error |= handle_error_with_force(e, &entry_path, options); @@ -389,7 +389,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt } // Recursively remove subdirectory using safe traversal - let child_dir_fd = match dir_fd.open_subdir(&entry_name) { + let child_dir_fd = match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) { Ok(fd) => fd, Err(e) => { // If we can't open the subdirectory for safe traversal, diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 467406b45fd..ac505c9ca90 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -22,7 +22,7 @@ use std::ffi::OsString; use walkdir::WalkDir; #[cfg(target_os = "linux")] -use crate::features::safe_traversal::DirFd; +use crate::features::safe_traversal::{DirFd, SymlinkBehavior}; use std::ffi::CString; use std::fs::Metadata; @@ -311,7 +311,7 @@ impl ChownExecutor { #[cfg(target_os = "linux")] let chown_result = if path.is_dir() { // For directories on Linux, use safe traversal from the start - match DirFd::open(path) { + match DirFd::open(path, SymlinkBehavior::Follow) { Ok(dir_fd) => self .safe_chown_dir(&dir_fd, path, &meta) .map(|_| String::new()), @@ -478,7 +478,7 @@ impl ChownExecutor { // Get metadata for the entry let follow = self.traverse_symlinks == TraverseSymlinks::All; - let meta = match dir_fd.metadata_at(&entry_name, follow) { + let meta = match dir_fd.metadata_at(&entry_name, follow.into()) { Ok(m) => m, Err(e) => { *ret = 1; @@ -506,7 +506,8 @@ impl ChownExecutor { let chown_uid = self.dest_uid; let chown_gid = self.dest_gid; - if let Err(e) = dir_fd.chown_at(&entry_name, chown_uid, chown_gid, follow_symlinks) + if let Err(e) = + dir_fd.chown_at(&entry_name, chown_uid, chown_gid, follow_symlinks.into()) { *ret = 1; if self.verbosity.level != VerbosityLevel::Silent { @@ -536,7 +537,7 @@ impl ChownExecutor { // Recurse into subdirectories if meta.is_dir() && (follow || !meta.file_type().is_symlink()) { - match dir_fd.open_subdir(&entry_name) { + match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) { Ok(subdir_fd) => { self.safe_traverse_dir(&subdir_fd, &entry_path, ret); } @@ -694,7 +695,7 @@ impl ChownExecutor { /// Try to open directory with error reporting #[cfg(target_os = "linux")] fn try_open_dir(&self, path: &Path) -> Option { - DirFd::open(path) + DirFd::open(path, SymlinkBehavior::Follow) .map_err(|e| { if self.verbosity.level != VerbosityLevel::Silent { show_error!("cannot access {}: {}", path.quote(), strip_errno(&e)); diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 05c456ad4ce..d58d4d9fa10 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -9,15 +9,16 @@ // Available on Unix // // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile -// spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod +// spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod mkdirat CREAT WRONLY ELOOP ENOTDIR #[cfg(test)] use std::os::unix::ffi::OsStringExt; use std::ffi::{CString, OsStr, OsString}; +use std::fs; use std::io; use std::os::unix::ffi::OsStrExt; -use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd}; +use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::path::{Path, PathBuf}; use nix::dir::Dir; @@ -29,6 +30,33 @@ use os_display::Quotable; use crate::translate; +/// Enum to specify symlink following behavior. +/// +/// This replaces boolean `follow_symlinks` parameters for better readability +/// at call sites. Instead of `open(path, true)`, use `open(path, SymlinkBehavior::Follow)`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum SymlinkBehavior { + /// Follow symlinks (resolve to their target) + #[default] + Follow, + /// Do not follow symlinks (operate on the symlink itself) + NoFollow, +} + +impl SymlinkBehavior { + /// Returns `true` if symlinks should be followed + #[inline] + pub fn should_follow(self) -> bool { + matches!(self, Self::Follow) + } +} + +impl From for SymlinkBehavior { + fn from(follow: bool) -> Self { + if follow { Self::Follow } else { Self::NoFollow } + } +} + // Custom error types for better error reporting #[derive(thiserror::Error, Debug)] pub enum SafeTraversalError { @@ -105,8 +133,15 @@ pub struct DirFd { impl DirFd { /// Open a directory and return a file descriptor - pub fn open(path: &Path) -> io::Result { - let flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC; + /// + /// # Arguments + /// * `path` - The path to the directory to open + /// * `symlink_behavior` - Whether to follow symlinks when opening + pub fn open(path: &Path, symlink_behavior: SymlinkBehavior) -> io::Result { + let mut flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC; + if !symlink_behavior.should_follow() { + flags |= OFlag::O_NOFOLLOW; + } let fd = nix::fcntl::open(path, flags, Mode::empty()).map_err(|e| { SafeTraversalError::OpenFailed { path: path.into(), @@ -117,10 +152,17 @@ impl DirFd { } /// Open a subdirectory relative to this directory - pub fn open_subdir(&self, name: &OsStr) -> io::Result { + /// + /// # Arguments + /// * `name` - The name of the subdirectory to open + /// * `symlink_behavior` - Whether to follow symlinks when opening + pub fn open_subdir(&self, name: &OsStr, symlink_behavior: SymlinkBehavior) -> io::Result { let name_cstr = CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; - let flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC; + let mut flags = OFlag::O_RDONLY | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC; + if !symlink_behavior.should_follow() { + flags |= OFlag::O_NOFOLLOW; + } let fd = openat(&self.fd, name_cstr.as_c_str(), flags, Mode::empty()).map_err(|e| { SafeTraversalError::OpenFailed { path: name.into(), @@ -131,11 +173,11 @@ impl DirFd { } /// Get raw stat data for a file relative to this directory - pub fn stat_at(&self, name: &OsStr, follow_symlinks: bool) -> io::Result { + pub fn stat_at(&self, name: &OsStr, symlink_behavior: SymlinkBehavior) -> io::Result { let name_cstr = CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; - let flags = if follow_symlinks { + let flags = if symlink_behavior.should_follow() { nix::fcntl::AtFlags::empty() } else { nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW @@ -152,8 +194,13 @@ impl DirFd { } /// Get metadata for a file relative to this directory - pub fn metadata_at(&self, name: &OsStr, follow_symlinks: bool) -> io::Result { - self.stat_at(name, follow_symlinks).map(Metadata::from_stat) + pub fn metadata_at( + &self, + name: &OsStr, + symlink_behavior: SymlinkBehavior, + ) -> io::Result { + self.stat_at(name, symlink_behavior) + .map(Metadata::from_stat) } /// Get metadata for this directory @@ -208,12 +255,12 @@ impl DirFd { name: &OsStr, uid: Option, gid: Option, - follow_symlinks: bool, + symlink_behavior: SymlinkBehavior, ) -> io::Result<()> { let name_cstr = CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; - let flags = if follow_symlinks { + let flags = if symlink_behavior.should_follow() { nix::fcntl::AtFlags::empty() } else { nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW @@ -239,8 +286,13 @@ impl DirFd { } /// Change mode of a file relative to this directory - pub fn chmod_at(&self, name: &OsStr, mode: u32, follow_symlinks: bool) -> io::Result<()> { - let flags = if follow_symlinks { + pub fn chmod_at( + &self, + name: &OsStr, + mode: u32, + symlink_behavior: SymlinkBehavior, + ) -> io::Result<()> { + let flags = if symlink_behavior.should_follow() { FchmodatFlags::FollowSymlink } else { FchmodatFlags::NoFollowSymlink @@ -267,6 +319,41 @@ impl DirFd { Ok(()) } + /// Create a directory relative to this directory + pub fn mkdir_at(&self, name: &OsStr, mode: u32) -> io::Result<()> { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + let mode = mode as libc::mode_t; + let fd = self.fd.as_raw_fd(); + + let result = unsafe { libc::mkdirat(fd, name_cstr.as_ptr(), mode) }; + if result == -1 { + let err = io::Error::last_os_error(); + return Err(SafeTraversalError::OpenFailed { + path: name.into(), + source: err, + } + .into()); + } + Ok(()) + } + + /// Open a file for writing relative to this directory + /// Creates the file if it doesn't exist, truncates if it does + pub fn open_file_at(&self, name: &OsStr) -> io::Result { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + let flags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_TRUNC | OFlag::O_CLOEXEC; + let mode = Mode::from_bits_truncate(0o666); // Default file permissions + + let fd: OwnedFd = openat(self.fd.as_fd(), name_cstr.as_c_str(), flags, mode) + .map_err(|e| io::Error::from_raw_os_error(e as i32))?; + + // Convert OwnedFd to raw fd and create File + let raw_fd = fd.into_raw_fd(); + Ok(unsafe { fs::File::from_raw_fd(raw_fd) }) + } + /// Create a DirFd from an existing file descriptor (takes ownership) pub fn from_raw_fd(fd: RawFd) -> io::Result { if fd < 0 { @@ -281,6 +368,152 @@ impl DirFd { } } +/// Find the deepest existing real directory ancestor for a path. +/// +/// Returns the existing ancestor path and a list of components that need to be created. +/// Uses `symlink_metadata` to detect symlinks - symlinks are NOT followed and are +/// treated as components that need to be created/replaced. +fn find_existing_ancestor(path: &Path) -> io::Result<(PathBuf, Vec)> { + let mut current = path.to_path_buf(); + let mut components: Vec = Vec::new(); + + loop { + // Use symlink_metadata to NOT follow symlinks + match fs::symlink_metadata(¤t) { + Ok(meta) => { + if meta.is_dir() && !meta.file_type().is_symlink() { + // Found a real directory (not a symlink to a directory) + components.reverse(); + return Ok((current, components)); + } + // It's a symlink, file, or other non-directory - treat as needing creation + // This ensures symlinks get replaced by open_or_create_subdir + if let Some(file_name) = current.file_name() { + components.push(file_name.to_os_string()); + } + if let Some(parent) = current.parent() { + if parent.as_os_str().is_empty() { + // Reached empty parent (for relative paths), use "." + components.reverse(); + return Ok((PathBuf::from("."), components)); + } + current = parent.to_path_buf(); + } else { + // Reached filesystem root + let root = if path.is_absolute() { + PathBuf::from("/") + } else { + PathBuf::from(".") + }; + components.reverse(); + return Ok((root, components)); + } + } + Err(e) if e.kind() == io::ErrorKind::NotFound => { + // Doesn't exist, record component and move up to parent + if let Some(file_name) = current.file_name() { + components.push(file_name.to_os_string()); + } + if let Some(parent) = current.parent() { + if parent.as_os_str().is_empty() { + // Reached empty parent (for relative paths), use "." + components.reverse(); + return Ok((PathBuf::from("."), components)); + } + current = parent.to_path_buf(); + } else { + // Reached filesystem root + let root = if path.is_absolute() { + PathBuf::from("/") + } else { + PathBuf::from(".") + }; + components.reverse(); + return Ok((root, components)); + } + } + Err(e) => return Err(e), + } + } +} + +/// Open or create a subdirectory using fd-based operations only. +/// +/// This is a helper function for `create_dir_all_safe` that handles a single +/// path component. If a symlink exists where a directory should be, it is +/// removed and replaced with a real directory. +/// +/// # Arguments +/// * `parent_fd` - The parent directory file descriptor +/// * `name` - The name of the subdirectory to open or create +/// * `mode` - The mode to use when creating a new directory +/// +/// # Returns +/// A DirFd for the subdirectory +fn open_or_create_subdir(parent_fd: &DirFd, name: &OsStr, mode: u32) -> io::Result { + match parent_fd.stat_at(name, SymlinkBehavior::NoFollow) { + Ok(stat) => { + let file_type = (stat.st_mode as libc::mode_t) & libc::S_IFMT; + match file_type { + libc::S_IFDIR => parent_fd.open_subdir(name, SymlinkBehavior::NoFollow), + libc::S_IFLNK => { + parent_fd.unlink_at(name, false)?; + parent_fd.mkdir_at(name, mode)?; + parent_fd.open_subdir(name, SymlinkBehavior::NoFollow) + } + _ => Err(io::Error::new( + io::ErrorKind::AlreadyExists, + format!( + "path component exists but is not a directory: {}", + name.display() + ), + )), + } + } + Err(e) if e.kind() == io::ErrorKind::NotFound => { + parent_fd.mkdir_at(name, mode)?; + parent_fd.open_subdir(name, SymlinkBehavior::NoFollow) + } + Err(e) => Err(e), + } +} + +/// Safely create all parent directories for a path using directory file descriptors. +/// This prevents symlink race conditions by anchoring all operations to directory fds. +/// +/// # Security +/// This function prevents TOCTOU race conditions by: +/// 1. Finding the deepest existing ancestor directory (path-based, but safe since it exists) +/// 2. Opening that ancestor with a file descriptor +/// 3. Creating all new directories using fd-based operations (mkdirat, openat with O_NOFOLLOW) +/// +/// Once we have a fd for an existing ancestor, all subsequent operations use that fd +/// as the anchor. If an attacker replaces a newly-created directory with a symlink, +/// our openat with O_NOFOLLOW will fail, preventing the attack. +/// +/// Existing symlinks in the path (like /var -> /private/var on macOS) are followed +/// when finding the ancestor, which is safe since they already exist. +/// +/// # Arguments +/// * `path` - The path to create directories for +/// * `mode` - The mode to use when creating new directories (e.g., 0o755). The actual +/// mode will be modified by the process umask. +/// +/// # Returns +/// A DirFd for the final created directory, or the first existing parent if +/// all directories already exist. +#[cfg(unix)] +pub fn create_dir_all_safe(path: &Path, mode: u32) -> io::Result { + let (existing_ancestor, components_to_create) = find_existing_ancestor(path)?; + let mut dir_fd = DirFd::open(&existing_ancestor, SymlinkBehavior::Follow)?; + + for component in &components_to_create { + dir_fd = open_or_create_subdir(&dir_fd, component.as_os_str(), mode)?; + } + + Ok(dir_fd) +} + impl AsRawFd for DirFd { fn as_raw_fd(&self) -> RawFd { self.fd.as_raw_fd() @@ -551,13 +784,13 @@ mod tests { #[test] fn test_dirfd_open_valid_directory() { let temp_dir = TempDir::new().unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); assert!(dir_fd.as_raw_fd() >= 0); } #[test] fn test_dirfd_open_nonexistent_directory() { - let result = DirFd::open("/nonexistent/path".as_ref()); + let result = DirFd::open("/nonexistent/path".as_ref(), SymlinkBehavior::Follow); assert!(result.is_err()); if let Err(e) = result { // The error should be the underlying io::Error @@ -573,7 +806,7 @@ mod tests { let file_path = temp_dir.path().join("test_file"); fs::write(&file_path, "test content").unwrap(); - let result = DirFd::open(&file_path); + let result = DirFd::open(&file_path, SymlinkBehavior::Follow); assert!(result.is_err()); } @@ -583,17 +816,19 @@ mod tests { let subdir = temp_dir.path().join("subdir"); fs::create_dir(&subdir).unwrap(); - let parent_fd = DirFd::open(temp_dir.path()).unwrap(); - let subdir_fd = parent_fd.open_subdir(OsStr::new("subdir")).unwrap(); + let parent_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let subdir_fd = parent_fd + .open_subdir(OsStr::new("subdir"), SymlinkBehavior::Follow) + .unwrap(); assert!(subdir_fd.as_raw_fd() >= 0); } #[test] fn test_dirfd_open_nonexistent_subdir() { let temp_dir = TempDir::new().unwrap(); - let parent_fd = DirFd::open(temp_dir.path()).unwrap(); + let parent_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); - let result = parent_fd.open_subdir(OsStr::new("nonexistent")); + let result = parent_fd.open_subdir(OsStr::new("nonexistent"), SymlinkBehavior::Follow); assert!(result.is_err()); } @@ -603,8 +838,10 @@ mod tests { let file_path = temp_dir.path().join("test_file"); fs::write(&file_path, "test content").unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); - let stat = dir_fd.stat_at(OsStr::new("test_file"), true).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let stat = dir_fd + .stat_at(OsStr::new("test_file"), SymlinkBehavior::Follow) + .unwrap(); assert!(stat.st_size > 0); assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFREG); @@ -619,21 +856,25 @@ mod tests { fs::write(&target_file, "target content").unwrap(); symlink(&target_file, &symlink_file).unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); // Follow symlinks - let stat_follow = dir_fd.stat_at(OsStr::new("link"), true).unwrap(); + let stat_follow = dir_fd + .stat_at(OsStr::new("link"), SymlinkBehavior::Follow) + .unwrap(); assert_eq!(stat_follow.st_mode & libc::S_IFMT, libc::S_IFREG); // Don't follow symlinks - let stat_nofollow = dir_fd.stat_at(OsStr::new("link"), false).unwrap(); + let stat_nofollow = dir_fd + .stat_at(OsStr::new("link"), SymlinkBehavior::NoFollow) + .unwrap(); assert_eq!(stat_nofollow.st_mode & libc::S_IFMT, libc::S_IFLNK); } #[test] fn test_dirfd_fstat() { let temp_dir = TempDir::new().unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); let stat = dir_fd.fstat().unwrap(); assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFDIR); @@ -648,7 +889,7 @@ mod tests { fs::write(&file1, "content1").unwrap(); fs::write(&file2, "content2").unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); let entries = dir_fd.read_dir().unwrap(); assert_eq!(entries.len(), 2); @@ -662,7 +903,7 @@ mod tests { let file_path = temp_dir.path().join("test_file"); fs::write(&file_path, "test content").unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); dir_fd.unlink_at(OsStr::new("test_file"), false).unwrap(); assert!(!file_path.exists()); @@ -674,7 +915,7 @@ mod tests { let subdir = temp_dir.path().join("empty_dir"); fs::create_dir(&subdir).unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); dir_fd.unlink_at(OsStr::new("empty_dir"), true).unwrap(); assert!(!subdir.exists()); @@ -683,7 +924,7 @@ mod tests { #[test] fn test_from_raw_fd() { let temp_dir = TempDir::new().unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); // Duplicate the fd first so we don't have ownership conflicts let dup_fd = nix::unistd::dup(&dir_fd).unwrap(); @@ -709,8 +950,10 @@ mod tests { let file_path = temp_dir.path().join("test_file"); fs::write(&file_path, "test content").unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); - let stat = dir_fd.stat_at(OsStr::new("test_file"), true).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let stat = dir_fd + .stat_at(OsStr::new("test_file"), SymlinkBehavior::Follow) + .unwrap(); let file_info = FileInfo::from_stat(&stat); assert_eq!(file_info.device(), stat.st_dev as u64); assert_eq!(file_info.inode(), stat.st_ino as u64); @@ -757,8 +1000,10 @@ mod tests { let file_path = temp_dir.path().join("test_file"); fs::write(&file_path, "test content with some length").unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); - let metadata = dir_fd.metadata_at(OsStr::new("test_file"), true).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let metadata = dir_fd + .metadata_at(OsStr::new("test_file"), SymlinkBehavior::Follow) + .unwrap(); assert_eq!(metadata.file_type(), FileType::RegularFile); assert!(metadata.size() > 0); @@ -771,7 +1016,7 @@ mod tests { #[test] fn test_metadata_directory() { let temp_dir = TempDir::new().unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); let metadata = dir_fd.metadata().unwrap(); assert_eq!(metadata.file_type(), FileType::Directory); @@ -782,9 +1027,9 @@ mod tests { fn test_path_with_null_byte() { let path_with_null = OsString::from_vec(b"test\0file".to_vec()); let temp_dir = TempDir::new().unwrap(); - let dir_fd = DirFd::open(temp_dir.path()).unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); - let result = dir_fd.open_subdir(&path_with_null); + let result = dir_fd.open_subdir(&path_with_null, SymlinkBehavior::Follow); assert!(result.is_err()); if let Err(e) = result { // Should be InvalidInput for null byte error @@ -794,7 +1039,10 @@ mod tests { #[test] fn test_error_chain() { - let result = DirFd::open("/nonexistent/deeply/nested/path".as_ref()); + let result = DirFd::open( + "/nonexistent/deeply/nested/path".as_ref(), + SymlinkBehavior::Follow, + ); assert!(result.is_err()); if let Err(e) = result { @@ -806,4 +1054,171 @@ mod tests { ); } } + + #[test] + fn test_mkdir_at_creates_directory() { + let temp_dir = TempDir::new().unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + + dir_fd.mkdir_at(OsStr::new("new_subdir"), 0o755).unwrap(); + + assert!(temp_dir.path().join("new_subdir").is_dir()); + } + + #[test] + fn test_mkdir_at_fails_if_exists() { + let temp_dir = TempDir::new().unwrap(); + let subdir = temp_dir.path().join("existing"); + fs::create_dir(&subdir).unwrap(); + + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let result = dir_fd.mkdir_at(OsStr::new("existing"), 0o755); + + assert!(result.is_err()); + } + + #[test] + fn test_open_file_at_creates_file() { + let temp_dir = TempDir::new().unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + + let mut file = dir_fd.open_file_at(OsStr::new("new_file.txt")).unwrap(); + use std::io::Write; + file.write_all(b"test content").unwrap(); + + let content = fs::read_to_string(temp_dir.path().join("new_file.txt")).unwrap(); + assert_eq!(content, "test content"); + } + + #[test] + fn test_open_file_at_truncates_existing() { + let temp_dir = TempDir::new().unwrap(); + let file_path = temp_dir.path().join("existing.txt"); + fs::write(&file_path, "old content that is longer").unwrap(); + + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + let mut file = dir_fd.open_file_at(OsStr::new("existing.txt")).unwrap(); + use std::io::Write; + file.write_all(b"new").unwrap(); + drop(file); + + let content = fs::read_to_string(&file_path).unwrap(); + assert_eq!(content, "new"); + } + + #[test] + fn test_create_dir_all_safe_creates_nested_dirs() { + let temp_dir = TempDir::new().unwrap(); + let nested_path = temp_dir.path().join("a/b/c"); + + let dir_fd = create_dir_all_safe(&nested_path, 0o755).unwrap(); + assert!(dir_fd.as_raw_fd() >= 0); + assert!(nested_path.is_dir()); + } + + #[test] + fn test_create_dir_all_safe_existing_path() { + let temp_dir = TempDir::new().unwrap(); + let existing_path = temp_dir.path().join("existing"); + fs::create_dir(&existing_path).unwrap(); + + let dir_fd = create_dir_all_safe(&existing_path, 0o755).unwrap(); + assert!(dir_fd.as_raw_fd() >= 0); + } + + #[test] + fn test_create_dir_all_safe_replaces_symlink() { + let temp_dir = TempDir::new().unwrap(); + let target_dir = temp_dir.path().join("target"); + fs::create_dir(&target_dir).unwrap(); + + // Create a symlink where we want to create a directory + let symlink_path = temp_dir.path().join("link_to_replace"); + symlink(&target_dir, &symlink_path).unwrap(); + assert!(symlink_path.is_symlink()); + + // create_dir_all_safe should replace the symlink with a real directory + let dir_fd = create_dir_all_safe(&symlink_path, 0o755).unwrap(); + assert!(dir_fd.as_raw_fd() >= 0); + + // Verify the symlink was replaced with a real directory + assert!(symlink_path.is_dir()); + assert!(!symlink_path.is_symlink()); + } + + #[test] + fn test_create_dir_all_safe_fails_on_file() { + let temp_dir = TempDir::new().unwrap(); + let file_path = temp_dir.path().join("file"); + fs::write(&file_path, "content").unwrap(); + + let result = create_dir_all_safe(&file_path, 0o755); + assert!(result.is_err()); + } + + #[test] + fn test_create_dir_all_safe_nested_symlink_in_path() { + let temp_dir = TempDir::new().unwrap(); + + // Create: parent/symlink -> target + // Then try to create: parent/symlink/subdir + let parent = temp_dir.path().join("parent"); + let target = temp_dir.path().join("target"); + fs::create_dir(&parent).unwrap(); + fs::create_dir(&target).unwrap(); + + let symlink_in_path = parent.join("link"); + symlink(&target, &symlink_in_path).unwrap(); + + // Try to create parent/link/subdir - the symlink should be replaced + let nested_path = symlink_in_path.join("subdir"); + let dir_fd = create_dir_all_safe(&nested_path, 0o755).unwrap(); + assert!(dir_fd.as_raw_fd() >= 0); + + // The symlink should have been replaced with a real directory + assert!(!symlink_in_path.is_symlink()); + assert!(symlink_in_path.is_dir()); + assert!(nested_path.is_dir()); + + // Target directory should not contain subdir (race attack prevented) + assert!(!target.join("subdir").exists()); + } + + #[test] + fn test_open_subdir_nofollow_fails_on_symlink() { + let temp_dir = TempDir::new().unwrap(); + let target = temp_dir.path().join("target"); + fs::create_dir(&target).unwrap(); + + let link = temp_dir.path().join("link"); + symlink(&target, &link).unwrap(); + + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + + // With follow_symlinks=true, should succeed + let result_follow = dir_fd.open_subdir(OsStr::new("link"), SymlinkBehavior::Follow); + assert!(result_follow.is_ok()); + + // With follow_symlinks=false, should fail (ELOOP or ENOTDIR) + let result_nofollow = dir_fd.open_subdir(OsStr::new("link"), SymlinkBehavior::NoFollow); + assert!(result_nofollow.is_err()); + } + + #[test] + fn test_open_nofollow_fails_on_symlink() { + let temp_dir = TempDir::new().unwrap(); + let target = temp_dir.path().join("target"); + fs::create_dir(&target).unwrap(); + + let link = temp_dir.path().join("link"); + symlink(&target, &link).unwrap(); + + // With follow_symlinks=true, should succeed + let result_follow = DirFd::open(&link, SymlinkBehavior::Follow); + assert!(result_follow.is_ok()); + + // With follow_symlinks=false, should fail + let result_nofollow = DirFd::open(&link, SymlinkBehavior::NoFollow); + assert!(result_nofollow.is_err()); + } } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 32cdbee8ca0..83e4353da58 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) helloworld nodir objdump n'source nconfined +// spell-checker:ignore (words) helloworld nodir objdump n'source nconfined testdir #[cfg(not(target_os = "openbsd"))] use filetime::FileTime; @@ -2567,3 +2567,94 @@ fn test_install_normal_file_replaces_symlink() { // Verify sensitive file was NOT modified assert_eq!(at.read("sensitive"), "important data"); } + +#[test] +#[cfg(unix)] +fn test_install_d_symlink_race_condition() { + // Test for symlink race condition fix (issue #10013) + // Verifies that pre-existing symlinks in path are handled safely + use std::os::unix::fs::symlink; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create test directories + at.mkdir("target"); + + // Create source file + at.write("source_file", "test content"); + + // Set up a pre-existing symlink attack scenario + at.mkdir_all("testdir/a"); + let intermediate_dir = at.plus("testdir/a/b"); + symlink(at.plus("target"), &intermediate_dir).unwrap(); + + // Run install -D which should detect and handle the symlink + let result = scene + .ucmd() + .arg("-D") + .arg(at.plus("source_file")) + .arg(at.plus("testdir/a/b/c/file")) + .run(); + + let wrong_location = at.plus("target/c/file"); + + // The critical assertion: file must NOT be in symlink target (race prevented) + assert!( + !wrong_location.exists(), + "RACE CONDITION NOT PREVENTED: File was created in symlink target" + ); + + // If the command succeeded, verify the file is in the correct location + if result.succeeded() { + assert!(at.file_exists("testdir/a/b/c/file")); + assert_eq!(at.read("testdir/a/b/c/file"), "test content"); + // The symlink should have been replaced with a real directory + assert!( + at.plus("testdir/a/b").is_dir() && !at.plus("testdir/a/b").is_symlink(), + "Intermediate path should be a real directory, not a symlink" + ); + } +} + +#[test] +#[cfg(unix)] +fn test_install_d_symlink_race_condition_concurrent() { + // Test pre-existing symlinks in intermediate paths are handled correctly + use std::os::unix::fs::symlink; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create test directories and source file using testing framework + at.mkdir("target2"); + at.write("source_file2", "test content 2"); + + // Set up intermediate directory with symlink + at.mkdir_all("testdir2/a"); + symlink(at.plus("target2"), at.plus("testdir2/a/b")).unwrap(); + + // Run install -D + scene + .ucmd() + .arg("-D") + .arg(at.plus("source_file2")) + .arg(at.plus("testdir2/a/b/c/file")) + .succeeds(); + + // Verify file was created at the intended destination + assert!(at.file_exists("testdir2/a/b/c/file")); + assert_eq!(at.read("testdir2/a/b/c/file"), "test content 2"); + + // Verify file was NOT created in symlink target + assert!( + !at.plus("target2/c/file").exists(), + "File should NOT be in symlink target location" + ); + + // Verify intermediate path is now a real directory + assert!( + at.plus("testdir2/a/b").is_dir() && !at.plus("testdir2/a/b").is_symlink(), + "Intermediate directory should be a real directory, not a symlink" + ); +}