From 3e557aa08cce66febdac3bcc15639f076424a8ec Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 21 Sep 2026 02:31:17 -0700 Subject: [PATCH 1/3] install: create leading directories in write-only directories --- Cargo.toml | 1 + src/uucore/src/lib/features/safe_traversal.rs | 102 +++++++++++++++++- tests/by-util/test_install.rs | 35 ++++++ 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bf6ca830d1..ad9441231a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -370,6 +370,7 @@ uucore = { workspace = true, features = [ "entries", "pipes", "process", + "safe-traversal", "signals", "utmpx", ] } diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 587fabf36c..ceae26b265 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -28,7 +28,7 @@ use std::time::SystemTime; use nix::dir::Dir; use nix::fcntl::{OFlag, openat}; use nix::libc; -use nix::sys::stat::{FchmodatFlags, FileStat, Mode, fchmodat, fstatat, mkdirat}; +use nix::sys::stat::{FchmodatFlags, FileStat, Mode, fchmodat, fstat, fstatat, mkdirat}; use nix::unistd::{Gid, Uid, UnlinkatFlags, fchown, fchownat, unlinkat}; use os_display::Quotable; @@ -146,6 +146,43 @@ const LARGEFILE: OFlag = OFlag::O_LARGEFILE; #[cfg(not(any(target_os = "linux", target_os = "android")))] const LARGEFILE: OFlag = OFlag::empty(); +/// Flag that opens a directory as an anchor for `*at` calls without read access. +/// +/// `mkdirat` and `openat` need write and execute on the anchor directory, but +/// opening it `O_RDONLY` also demands read, which fails on write-only +/// directories where GNU succeeds. `O_PATH` (Linux) and `O_SEARCH` (POSIX +/// 2008) both yield a descriptor that anchors `*at` calls without reading. +/// Such a descriptor cannot list directory entries. +#[cfg(any(target_os = "linux", target_os = "android"))] +const SEARCH_ONLY: Option = Some(OFlag::O_PATH); +#[cfg(any( + target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "netbsd", + target_os = "illumos", + target_os = "solaris" +))] +const SEARCH_ONLY: Option = Some(OFlag::O_SEARCH); +#[cfg(not(any( + target_os = "linux", + target_os = "android", + target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "netbsd", + target_os = "illumos", + target_os = "solaris" +)))] +const SEARCH_ONLY: Option = None; + +/// Whether this platform can anchor `*at` calls on a directory it may not read. +/// +/// Where it cannot, creating an entry inside a write-only directory fails with +/// `EACCES` instead of succeeding the way `mkdir` does (OpenBSD, for example, +/// has neither `O_PATH` nor `O_SEARCH`). +pub const SEARCH_ONLY_SUPPORTED: bool = SEARCH_ONLY.is_some(); + impl DirFd { /// Open a directory and return a file descriptor /// @@ -166,6 +203,44 @@ impl DirFd { Ok(Self { fd }) } + /// Open a directory to anchor `*at` calls, following symlinks. + /// + /// Falls back to a search-only descriptor when the directory denies read + /// access, so that creating entries in a write-only directory works the + /// way it does with `mkdir`. The returned descriptor is only guaranteed to + /// support `*at` calls; it may not be able to list directory entries. + /// + /// Only symlink-following opens get the fallback. `O_PATH` ignores both + /// `O_DIRECTORY` and `O_NOFOLLOW`, so a search-only descriptor cannot + /// carry the "this must not be a symlink" guarantee that the traversal + /// relies on elsewhere. + pub fn open_anchor(path: &Path) -> io::Result { + let denied = match Self::open(path, SymlinkBehavior::Follow) { + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => e, + result => return result, + }; + let Some(search_only) = SEARCH_ONLY else { + return Err(denied); + }; + + let flags = search_only | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC | LARGEFILE; + let fd = nix::fcntl::open(path, flags, Mode::empty()).map_err(|_| denied)?; + let this = Self { fd }; + + // Linux honours neither O_DIRECTORY nor O_NOFOLLOW together with + // O_PATH, so a regular file opens just as happily as a directory here. + // Check what we actually got. + let stat = fstat(&this.fd).map_err(|e| SafeTraversalError::StatFailed { + path: path.into(), + source: io::Error::from_raw_os_error(e as i32), + })?; + if (stat.st_mode as libc::mode_t) & libc::S_IFMT == libc::S_IFDIR { + Ok(this) + } else { + Err(io::Error::from_raw_os_error(libc::ENOTDIR)) + } + } + /// Open a subdirectory relative to this directory /// /// # Arguments @@ -225,7 +300,7 @@ impl DirFd { /// Get raw stat data for this directory pub fn fstat(&self) -> io::Result { - let stat = nix::sys::stat::fstat(&self.fd).map_err(|e| SafeTraversalError::StatFailed { + let stat = fstat(&self.fd).map_err(|e| SafeTraversalError::StatFailed { path: translate!("safe-traversal-current-directory").into(), source: io::Error::from_raw_os_error(e as i32), })?; @@ -651,7 +726,7 @@ fn open_or_create_subdir(parent_fd: &DirFd, name: &OsStr, mode: u32) -> io::Resu #[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)?; + let mut dir_fd = DirFd::open_anchor(&existing_ancestor)?; for component in &components_to_create { dir_fd = open_or_create_subdir(&dir_fd, component.as_os_str(), mode)?; @@ -988,6 +1063,7 @@ mod tests { use super::*; use std::fs; use std::os::unix::fs::MetadataExt; + use std::os::unix::fs::PermissionsExt; use std::os::unix::fs::symlink; use std::os::unix::io::IntoRawFd; use tempfile::TempDir; @@ -1413,6 +1489,26 @@ mod tests { assert!(nested_path.is_dir()); } + #[test] + fn test_create_dir_all_safe_in_write_only_dir() { + if Uid::effective().is_root() { + // root ignores the permission bits this test depends on + return; + } + let temp_dir = TempDir::new().unwrap(); + let write_only = temp_dir.path().join("wx"); + fs::create_dir(&write_only).unwrap(); + fs::set_permissions(&write_only, fs::Permissions::from_mode(0o300)).unwrap(); + + // mkdir needs write and execute, not read: an unreadable parent must + // not stop us, the way it does not stop GNU. + let nested = write_only.join("a/b"); + create_dir_all_safe(&nested, 0o755).unwrap(); + + fs::set_permissions(&write_only, fs::Permissions::from_mode(0o755)).unwrap(); + assert!(nested.is_dir()); + } + #[test] fn test_create_dir_all_safe_existing_path() { let temp_dir = TempDir::new().unwrap(); diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 6832c6edbd..2483b21f64 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -2906,6 +2906,41 @@ fn test_install_d_dangling_symlink_in_path_errors() { ); } +#[test] +#[cfg(unix)] +fn test_install_d_leading_dirs_in_write_only_directory() { + // mkdir needs write and execute on the parent, not read, so -D must be + // able to create leading directories inside a directory it cannot read. + use std::os::unix::fs::PermissionsExt; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + if geteuid().is_root() { + println!("Test skipped; root ignores directory permissions"); + return; + } + if !uucore::safe_traversal::SEARCH_ONLY_SUPPORTED { + println!("Test skipped; platform cannot anchor on an unreadable directory"); + return; + } + + at.write("file.txt", "hello"); + at.mkdir("wx"); + fs::set_permissions(at.plus("wx"), fs::Permissions::from_mode(0o300)).unwrap(); + + scene + .ucmd() + .args(&["-D", "file.txt", "wx/a/b/file.txt"]) + .succeeds(); + + fs::set_permissions(at.plus("wx"), fs::Permissions::from_mode(0o755)).unwrap(); + assert_eq!( + fs::read_to_string(at.plus("wx/a/b/file.txt")).unwrap(), + "hello" + ); +} + #[test] #[cfg(target_os = "linux")] fn test_install_set_owner_nonexistent_uid_and_gid() { From 7d69d94cfa9162740e827c0c408cdaaff81cb6ac Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Wed, 30 Sep 2026 23:52:09 -0700 Subject: [PATCH 2/3] tests/install: skip the write-only -D test where safe_traversal is not built --- tests/by-util/test_install.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 2483b21f64..69bd7ffb30 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -2907,7 +2907,10 @@ fn test_install_d_dangling_symlink_in_path_errors() { } #[test] -#[cfg(unix)] +#[cfg(all( + unix, + not(any(target_os = "aix", target_os = "hurd", target_os = "redox")) +))] fn test_install_d_leading_dirs_in_write_only_directory() { // mkdir needs write and execute on the parent, not read, so -D must be // able to create leading directories inside a directory it cannot read. From 57c03da2fb7ed9fe07e19dbd196e04625dc7ea22 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Thu, 1 Oct 2026 00:04:07 -0700 Subject: [PATCH 3/3] uucore: drop the O_PATH stat check, O_DIRECTORY already rejects non-directories Also skip the write-only unit test where neither O_PATH nor O_SEARCH exists. --- src/uucore/src/lib/features/safe_traversal.rs | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index ceae26b265..05676c70f4 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -209,11 +209,6 @@ impl DirFd { /// access, so that creating entries in a write-only directory works the /// way it does with `mkdir`. The returned descriptor is only guaranteed to /// support `*at` calls; it may not be able to list directory entries. - /// - /// Only symlink-following opens get the fallback. `O_PATH` ignores both - /// `O_DIRECTORY` and `O_NOFOLLOW`, so a search-only descriptor cannot - /// carry the "this must not be a symlink" guarantee that the traversal - /// relies on elsewhere. pub fn open_anchor(path: &Path) -> io::Result { let denied = match Self::open(path, SymlinkBehavior::Follow) { Err(e) if e.kind() == io::ErrorKind::PermissionDenied => e, @@ -224,21 +219,9 @@ impl DirFd { }; let flags = search_only | OFlag::O_DIRECTORY | OFlag::O_CLOEXEC | LARGEFILE; - let fd = nix::fcntl::open(path, flags, Mode::empty()).map_err(|_| denied)?; - let this = Self { fd }; - - // Linux honours neither O_DIRECTORY nor O_NOFOLLOW together with - // O_PATH, so a regular file opens just as happily as a directory here. - // Check what we actually got. - let stat = fstat(&this.fd).map_err(|e| SafeTraversalError::StatFailed { - path: path.into(), - source: io::Error::from_raw_os_error(e as i32), - })?; - if (stat.st_mode as libc::mode_t) & libc::S_IFMT == libc::S_IFDIR { - Ok(this) - } else { - Err(io::Error::from_raw_os_error(libc::ENOTDIR)) - } + nix::fcntl::open(path, flags, Mode::empty()) + .map(|fd| Self { fd }) + .map_err(|_| denied) } /// Open a subdirectory relative to this directory @@ -1491,8 +1474,9 @@ mod tests { #[test] fn test_create_dir_all_safe_in_write_only_dir() { - if Uid::effective().is_root() { - // root ignores the permission bits this test depends on + // root ignores the permission bits this test depends on, and without + // O_PATH or O_SEARCH the walk cannot anchor on an unreadable directory + if Uid::effective().is_root() || !SEARCH_ONLY_SUPPORTED { return; } let temp_dir = TempDir::new().unwrap();