From 028b3366be0a5590237ab474669d854156eb4dcb Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:31:10 -0700 Subject: [PATCH 1/5] uucore: add DirFd::open_anchor for write-only directories Open a directory readable, and on EACCES retry with O_PATH or O_SEARCH, which anchor *at calls without read access. The targets with either flag are named by the has_o_path and has_o_search cfg aliases. If the retry fails, its own error is returned. --- Cargo.lock | 1 + fuzz/Cargo.lock | 1 + src/uucore/Cargo.toml | 3 + src/uucore/build.rs | 19 ++++ src/uucore/src/lib/features/safe_traversal.rs | 102 ++++++++++++++++++ 5 files changed, 126 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 6b7171ecbf2..45b88919d99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4418,6 +4418,7 @@ dependencies = [ "blake2b_simd", "blake3", "bstr", + "cfg_aliases", "clap", "crc-fast", "data-encoding", diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 8ec03377f8d..5c40433753c 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -1833,6 +1833,7 @@ dependencies = [ "bigdecimal", "blake2b_simd", "blake3", + "cfg_aliases", "clap", "crc-fast", "data-encoding", diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index e5a178879ac..eb7ce9d5fa0 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -84,6 +84,9 @@ fluent-syntax = { workspace = true } unic-langid = { workspace = true } thiserror = { workspace = true } +[build-dependencies] +cfg_aliases = { workspace = true } + [dev-dependencies] tempfile = { workspace = true } diff --git a/src/uucore/build.rs b/src/uucore/build.rs index a9a7d09e6b6..cb97fd3085a 100644 --- a/src/uucore/build.rs +++ b/src/uucore/build.rs @@ -3,12 +3,31 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// spell-checker:ignore tvos watchos visionos + +use cfg_aliases::cfg_aliases; use std::env; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; pub fn main() -> Result<(), Box> { + cfg_aliases! { + // Directory open flags that grant search but not read access. + has_o_path: { any(target_os = "linux", target_os = "android") }, + has_o_search: { any( + target_os = "macos", + target_os = "ios", + target_os = "tvos", + target_os = "watchos", + target_os = "visionos", + target_os = "freebsd", + target_os = "netbsd", + target_os = "illumos", + target_os = "solaris" + ) }, + } + let out_dir = env::var("OUT_DIR")?; let mut embedded_file = File::create(Path::new(&out_dir).join("embedded_locales.rs"))?; diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index b8bd8183fb4..f6b1a11bd95 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -231,6 +231,23 @@ 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(has_o_path)] +const SEARCH_ONLY: Option = Some(OFlag::O_PATH); +#[cfg(has_o_search)] +const SEARCH_ONLY: Option = Some(OFlag::O_SEARCH); +/// Neither flag exists here (OpenBSD, for example), so creating an entry +/// inside a write-only directory fails with `EACCES` instead of succeeding the +/// way `mkdir` does. +#[cfg(not(any(has_o_path, has_o_search)))] +const SEARCH_ONLY: Option = None; + impl DirFd { /// Open a directory and return a file descriptor /// @@ -251,6 +268,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. + pub fn open_anchor(path: &Path) -> io::Result { + match Self::open(path, SymlinkBehavior::Follow) { + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + Self::open_search_only(path, e) + } + result => result, + } + } + + /// Retry a readable open of `path` that failed with `denied` using a + /// search-only descriptor. + /// + /// If this open fails as well, its own error is returned. The readable + /// open only adds a read-permission check, so a different error here + /// (`ENOENT`, `ELOOP`, `ENOTDIR`) means `path` changed between the two + /// calls and describes what is there now. Platforms without a search-only + /// flag return `denied` unchanged. + fn open_search_only(path: &Path, denied: io::Error) -> io::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(|e| { + SafeTraversalError::OpenFailed { + path: path.into(), + source: io::Error::from_raw_os_error(e as i32), + } + })?; + Ok(Self { fd }) + } + /// Open a subdirectory relative to this directory /// /// # Arguments @@ -1517,6 +1572,53 @@ mod tests { assert!(nested_path.is_dir()); } + #[test] + #[cfg(any(has_o_path, has_o_search))] + fn test_open_anchor_in_write_only_dir() { + use std::os::unix::fs::PermissionsExt; + + 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(); + + // Creating entries needs write and execute, not read. + let dir_fd = DirFd::open_anchor(&write_only).unwrap(); + dir_fd.mkdir_at(OsStr::new("sub"), 0o755).unwrap(); + dir_fd.open_file_at(OsStr::new("file")).unwrap(); + + fs::set_permissions(&write_only, fs::Permissions::from_mode(0o755)).unwrap(); + assert!(write_only.join("sub").is_dir()); + assert!(write_only.join("file").is_file()); + } + + #[test] + #[cfg(any(has_o_path, has_o_search))] + fn test_open_search_only_keeps_its_own_error() { + // If the path changes after the readable open was denied, report what + // the search-only open found, not the earlier EACCES. + let temp_dir = TempDir::new().unwrap(); + let missing = temp_dir.path().join("missing"); + let looping = temp_dir.path().join("loop"); + symlink(&looping, &looping).unwrap(); + let file = temp_dir.path().join("file"); + fs::write(&file, "").unwrap(); + + for (path, errno) in [ + (&missing, libc::ENOENT), + (&looping, libc::ELOOP), + (&file, libc::ENOTDIR), + ] { + let denied = io::Error::from_raw_os_error(libc::EACCES); + let err = DirFd::open_search_only(path, denied).err().unwrap(); + assert_eq!(err.raw_os_error(), Some(errno), "{}", path.display()); + } + } + #[test] fn test_create_dir_all_safe_existing_path() { let temp_dir = TempDir::new().unwrap(); From 510d7f75af974ee4227e58a05a905a1acf8c03e5 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:44:11 -0700 Subject: [PATCH 2/5] uucore: share replace_link's atomic replace Move the temporary name and renameat logic of replace_link into replace_entry_at, which creates the entry through a callback relative to an open directory, so other callers can replace entries the same way. The parent is now opened with DirFd::open_anchor, so ln -sf replacing a link in a directory with write and search but no read permission works, as it does with GNU, instead of failing with EACCES. --- src/uucore/Cargo.toml | 2 +- src/uucore/locales/en-US.ftl | 1 + src/uucore/locales/fr-FR.ftl | 1 + src/uucore/src/lib/features/fs.rs | 137 ++++++++++++++++++++---------- tests/by-util/test_ln.rs | 31 +++++++ 5 files changed, 128 insertions(+), 44 deletions(-) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index eb7ce9d5fa0..894d4b1efb3 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -159,7 +159,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc", "rustix/fs", "rustix/process"] extendedbigdecimal = ["bigdecimal", "num-traits"] fast-inc = [] -fs = ["dunce", "libc", "rustix/fs", "rustix/std", "windows-sys"] +fs = ["dunce", "libc", "rustix/fs", "rustix/std", "safe-traversal", "windows-sys"] fsext = ["libc", "windows-sys", "bstr", "wide"] fsxattr = ["xattr", "itertools", "libc"] hardware = [] diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index d4aa0eb952d..02fc4145cfa 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -35,6 +35,7 @@ error-invalid-argument = Invalid argument error-is-a-directory-text = Is a directory error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Too many levels of symbolic links +error-no-unique-temp-name = no unique temporary name available in the destination directory # Common actions action-copying = copying diff --git a/src/uucore/locales/fr-FR.ftl b/src/uucore/locales/fr-FR.ftl index 2b4ea26658c..0fa9d009c07 100644 --- a/src/uucore/locales/fr-FR.ftl +++ b/src/uucore/locales/fr-FR.ftl @@ -34,6 +34,7 @@ error-invalid-argument = Argument invalide error-is-a-directory-text = Est un répertoire error-is-a-directory = { $file }: { error-is-a-directory-text } error-too-many-symlink-levels = Trop de niveaux de liens symboliques +error-no-unique-temp-name = aucun nom temporaire unique disponible dans le répertoire de destination # Actions communes action-copying = copie diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 045ae3a3cef..5f99214edbe 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -1107,16 +1107,7 @@ pub fn get_filename(file: &Path) -> Option<&str> { pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> { #[cfg(all(unix, not(target_os = "redox")))] { - use rustix::fs::{AtFlags, CWD, Mode, OFlags, openat, renameat, unlinkat}; - use std::ffi::OsStr; - use std::io::Read; - use std::os::unix::ffi::OsStrExt; - - // GNU's template is `CuXXXXXX`: a 2-char prefix plus 6 random chars - // from a 62-char alphabet. The ~3% modulo bias per slot is irrelevant - // for an 8-char unguessability budget. - const ALPHABET: &[u8; 62] = - b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + use rustix::fs::CWD; match link_at(target, CWD, dest.as_os_str(), symbolic) { Err(e) if e.kind() == ErrorKind::AlreadyExists => {} @@ -1132,39 +1123,10 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> .ok_or_else(|| Error::new(ErrorKind::InvalidInput, "invalid link path"))?; // No NOFOLLOW: the parent may be a symlink to a directory, which the // create attempt above already followed. - let dir = openat( - CWD, - parent, - OFlags::DIRECTORY | OFlags::RDONLY | OFlags::CLOEXEC, - Mode::empty(), - )?; - let mut urandom = fs::File::open("/dev/urandom")?; - - for _ in 0..32 { - let mut name = *b"Cu------"; - let mut raw = [0u8; 6]; - urandom.read_exact(&mut raw)?; - for (slot, byte) in name[2..].iter_mut().zip(raw) { - *slot = ALPHABET[byte as usize % ALPHABET.len()]; - } - let tmp = OsStr::from_bytes(&name); - - match link_at(target, &dir, tmp, symbolic) { - Ok(()) => { - let renamed = renameat(&dir, tmp, &dir, basename); - // Renaming onto an existing link to the same inode is a - // no-op, which leaves the temp behind. - let _ = unlinkat(&dir, tmp, AtFlags::empty()); - return renamed.map_err(Into::into); - } - Err(e) if e.kind() == ErrorKind::AlreadyExists => {} - Err(e) => return Err(e), - } - } - Err(Error::new( - ErrorKind::AlreadyExists, - "no unique temporary name available in the destination directory", - )) + let dir = crate::safe_traversal::DirFd::open_anchor(parent)?; + replace_existing_entry_at(&dir, basename, |dir, name| { + link_at(target, dir, name, symbolic) + }) } #[cfg(any(windows, target_os = "redox", target_os = "wasi"))] { @@ -1180,6 +1142,95 @@ pub fn replace_link(target: &Path, dest: &Path, symbolic: bool) -> IOResult<()> } } +/// Create the entry `name` in `dir` with `create`, replacing an entry already +/// there. +/// +/// Never unlinks the existing entry first, which would briefly free the name +/// for another user to claim. Try the create; if the name is taken, create the +/// entry under a random temporary name in `dir` and `renameat(2)` it over. +/// `create` must fail with `EEXIST` when its name is taken, must not leave an +/// entry behind when it fails otherwise, and must not create a directory. +/// +/// # Errors +/// +/// Returns an error if `create` or the rename fails, or if no unique temporary +/// name is available. +#[cfg(all(unix, not(target_os = "redox")))] +pub fn replace_entry_at( + dir: &D, + name: &OsStr, + mut create: impl FnMut(&D, &OsStr) -> IOResult<()>, +) -> IOResult<()> { + match create(dir, name) { + Err(e) if e.kind() == ErrorKind::AlreadyExists => {} + res => return res, + } + replace_existing_entry_at(dir, name, create) +} + +/// [`replace_entry_at`] once creating `name` itself failed with `EEXIST`. +#[cfg(all(unix, not(target_os = "redox")))] +fn replace_existing_entry_at( + dir: &D, + name: &OsStr, + create: impl FnMut(&D, &OsStr) -> IOResult<()>, +) -> IOResult<()> { + use rustix::fs::{AtFlags, renameat, unlinkat}; + + let ((), tmp) = create_temp_at(dir, create)?; + let renamed = renameat(dir, &tmp, dir, name); + // Renaming onto an existing link to the same inode is a no-op, which + // leaves the temp behind. + let _ = unlinkat(dir, &tmp, AtFlags::empty()); + renamed.map_err(Into::into) +} + +/// Create an entry in `dir` under a random name that is not taken, and return +/// what `create` returned along with the name. +/// +/// `create` must fail with `EEXIST` when its name is taken; another name is +/// tried then. +/// +/// # Errors +/// +/// Returns an error if `create` fails otherwise, or if no unique name is +/// available. +#[cfg(all(unix, not(target_os = "redox")))] +pub fn create_temp_at( + dir: &D, + mut create: impl FnMut(&D, &OsStr) -> IOResult, +) -> IOResult<(T, OsString)> { + use std::io::Read; + use std::os::unix::ffi::OsStrExt; + + // GNU's template is `CuXXXXXX`: a 2-char prefix plus 6 random chars + // from a 62-char alphabet. The ~3% modulo bias per slot is irrelevant + // for an 8-char unguessability budget. + const ALPHABET: &[u8; 62] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + + let mut urandom = fs::File::open("/dev/urandom")?; + + for _ in 0..32 { + let mut tmp = *b"Cu------"; + let mut raw = [0u8; 6]; + urandom.read_exact(&mut raw)?; + for (slot, byte) in tmp[2..].iter_mut().zip(raw) { + *slot = ALPHABET[byte as usize % ALPHABET.len()]; + } + let tmp = OsStr::from_bytes(&tmp); + + match create(dir, tmp) { + Ok(created) => return Ok((created, tmp.to_owned())), + Err(e) if e.kind() == ErrorKind::AlreadyExists => {} + Err(e) => return Err(e), + } + } + Err(Error::new( + ErrorKind::AlreadyExists, + translate!("error-no-unique-temp-name"), + )) +} + /// `symlinkat`/`linkat` relative to an open directory. #[cfg(all(unix, not(target_os = "redox")))] fn link_at(target: &Path, dir: Fd, name: &OsStr, symbolic: bool) -> IOResult<()> { diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index f53028cdb4c..212b7241a68 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -3,6 +3,8 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +// spell-checker:ignore dirlink + #![allow(clippy::similar_names)] use std::path::PathBuf; @@ -114,6 +116,35 @@ fn test_symlink_overwrite_force() { assert_eq!(at.resolve_link(link), file_b); } +/// Replacing a link needs write and search permission on its directory, like +/// creating one, not read permission. +#[test] +#[cfg(any( + target_os = "linux", + target_os = "android", + target_os = "macos", + target_os = "freebsd", + target_os = "netbsd" +))] +fn test_symlink_overwrite_force_in_write_search_only_dir() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::PermissionsExt; + + if rustix::process::geteuid().is_root() { + return; + } + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir"); + at.symlink_file("old", "dir/link"); + set_permissions(at.plus("dir"), Permissions::from_mode(0o300)).unwrap(); + + let result = ucmd.args(&["-sf", "new", "dir/link"]).run(); + + set_permissions(at.plus("dir"), Permissions::from_mode(0o755)).unwrap(); + result.success(); + assert_eq!(at.resolve_link("dir/link"), "new"); +} + /// A forced replace must be atomic, so a concurrent creator always loses. /// Fails reliably if unlink-then-create ever comes back. #[test] From 5d10e9b217cb339a9a404e879d2e1406b4e2d781 Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Tue, 6 Oct 2026 00:57:45 -0700 Subject: [PATCH 3/5] uucore: format the fs feature list for taplo --- src/uucore/Cargo.toml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 894d4b1efb3..aea517fefe0 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -159,7 +159,14 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc", "rustix/fs", "rustix/process"] extendedbigdecimal = ["bigdecimal", "num-traits"] fast-inc = [] -fs = ["dunce", "libc", "rustix/fs", "rustix/std", "safe-traversal", "windows-sys"] +fs = [ + "dunce", + "libc", + "rustix/fs", + "rustix/std", + "safe-traversal", + "windows-sys", +] fsext = ["libc", "windows-sys", "bstr", "wide"] fsxattr = ["xattr", "itertools", "libc"] hardware = [] From 0fb05fddd23258bab3f69cc920a29c37b7081a7b Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Mon, 5 Oct 2026 18:51:05 -0700 Subject: [PATCH 4/5] uucore: add DirFd::mknod_at Create fifos, sockets and device nodes relative to an open directory. nix and rustix do not provide mknodat on Apple targets, so this calls libc directly. --- src/uucore/src/lib/features/safe_traversal.rs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index f6b1a11bd95..a0c1d6d08b5 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -6,7 +6,7 @@ // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile // spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod mkdirat CREAT WRONLY ELOOP ENOTDIR EXCL EEXIST // spell-checker:ignore atimensec mtimensec ctimensec opath chmods fakeroot fakechroot EOVERFLOW chowned chmoded -// spell-checker:ignore LARGEFILE getdents atim mtim ctim statat +// spell-checker:ignore LARGEFILE getdents atim mtim ctim statat mknodat // Safe directory traversal using openat() and related syscalls // This module provides TOCTOU-safe filesystem operations for recursive traversal @@ -599,6 +599,32 @@ impl DirFd { Ok(()) } + /// Create a fifo, socket, or device node relative to this directory + /// + /// `mode` holds the file type bits as well as the permissions, as for + /// `mknod(2)`, and the umask applies. Fails with `EEXIST` if the name + /// already exists. + pub fn mknod_at(&self, name: &OsStr, mode: u32, dev: u64) -> io::Result<()> { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + // Neither nix nor rustix provides `mknodat` on Apple targets. + // SAFETY: `name_cstr` is NUL-terminated and outlives the call, and + // `self.fd` is an open directory descriptor. + let res = unsafe { + libc::mknodat( + self.fd.as_raw_fd(), + name_cstr.as_ptr(), + mode as libc::mode_t, + dev as libc::dev_t, + ) + }; + if res == 0 { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } + } + /// Create a file for writing relative to this directory /// Fails with `EEXIST` if the name already exists pub fn open_file_at(&self, name: &OsStr) -> io::Result { @@ -1506,6 +1532,23 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_mknod_at_creates_fifo_and_refuses_existing() { + use std::os::unix::fs::FileTypeExt; + + let temp_dir = TempDir::new().unwrap(); + let dir_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap(); + // S_IFIFO | 0o600 + let fifo = 0o010_600; + + dir_fd.mknod_at(OsStr::new("fifo"), fifo, 0).unwrap(); + + let metadata = fs::symlink_metadata(temp_dir.path().join("fifo")).unwrap(); + assert!(metadata.file_type().is_fifo()); + let err = dir_fd.mknod_at(OsStr::new("fifo"), fifo, 0).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + } + #[test] fn test_open_file_at_creates_file() { use std::io::Write; From 25c92494ce0702271756860e7a27d141ca65e46c Mon Sep 17 00:00:00 2001 From: Jake Abendroth Date: Wed, 7 Oct 2026 01:17:42 -0700 Subject: [PATCH 5/5] mv: recreate special files when moving across devices A cross-device move of a socket or device node went through the regular file copy, which removed the destination and then failed to open the source. A fifo also removed the destination before creating the new one, and lost its mode. Fifos, sockets and device nodes are now created with their mode and ownership in a private directory next to the destination and renamed over it, so the destination is kept if the node cannot be created, and the ownership and mode cannot land on another file linked over the destination name meanwhile. The private directory is made 0700 by name right after it is created, before it is opened, without changing the umask; the check after the open rejects a directory moved there in between. As for regular files, setuid and setgid are dropped when the ownership cannot be kept. Fixes #13145 --- src/uu/mv/Cargo.toml | 2 +- src/uu/mv/src/mv.rs | 130 ++++++++++++++++++-- tests/by-util/test_mv.rs | 252 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 370 insertions(+), 14 deletions(-) diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index f446311a32f..018a4279d1d 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -42,7 +42,7 @@ windows-sys = { workspace = true, features = [ ] } [target.'cfg(unix)'.dependencies] -rustix = { workspace = true, features = ["fs"] } +rustix = { workspace = true, features = ["fs", "process"] } [[bin]] name = "mv" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 68cc9c21887..4614267b149 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -59,6 +59,8 @@ use uucore::fs::{ target_os = "netbsd" ))] use uucore::fsxattr; +#[cfg(all(unix, not(target_os = "redox")))] +use uucore::safe_traversal::{DirFd, SymlinkBehavior}; #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] use uucore::selinux::set_selinux_security_context; use uucore::translate; @@ -924,13 +926,17 @@ fn is_directory_not_empty_error(err: &io::Error) -> bool { err.kind() == io::ErrorKind::DirectoryNotEmpty } +/// Fifos, sockets and device nodes are recreated rather than copied. #[cfg(unix)] -fn is_fifo(filetype: fs::FileType) -> bool { +fn is_special_file(filetype: fs::FileType) -> bool { filetype.is_fifo() + || filetype.is_socket() + || filetype.is_block_device() + || filetype.is_char_device() } #[cfg(not(unix))] -fn is_fifo(_filetype: fs::FileType) -> bool { +fn is_special_file(_filetype: fs::FileType) -> bool { false } @@ -983,8 +989,8 @@ fn rename_with_fallback( { rename_dir_fallback(from, to, display_manager, verbose) } - } else if is_fifo(file_type) { - rename_fifo_fallback(from, to) + } else if is_special_file(file_type) { + rename_special_fallback(from, to, &metadata) } else { #[cfg(unix)] { @@ -1002,15 +1008,115 @@ fn rename_with_fallback( }) } -/// Replace the destination with a new pipe with the same name as the source. +/// Replace the destination with a new special file like the source. #[cfg(unix)] -fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { - if to.try_exists()? { +fn rename_special_fallback(from: &Path, to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + copy_special_file(to, metadata)?; + fs::remove_file(from) +} + +/// Create the fifo, socket or device node that `metadata` describes at `to`, +/// with its ownership and permissions. +/// +/// An entry at `to` is replaced atomically, so it is kept if the node cannot +/// be created. +#[cfg(all(unix, not(target_os = "redox")))] +fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + let parent = to + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let name = to.file_name().ok_or(io::ErrorKind::InvalidInput)?; + // Follows symlinks in `parent`, as creating the node by path would. + let dir = DirFd::open_anchor(parent)?; + create_special_file_at(&dir, name, metadata) +} + +/// Create the special file that `metadata` describes as `name` in `dir`, with +/// its ownership and permissions, replacing an entry already there. +/// +/// The node gets its ownership and mode inside a new private directory in +/// `dir` and is then renamed into place. In `dir` itself, whoever else can +/// write there could link another file over the name between those calls. +#[cfg(all(unix, not(target_os = "redox")))] +fn create_special_file_at( + dir: &DirFd, + name: &std::ffi::OsStr, + metadata: &fs::Metadata, +) -> io::Result<()> { + use rustix::fs::renameat; + use rustix::process::geteuid; + use std::os::unix::fs::MetadataExt; + + let (staging, staging_name) = uucore::fs::create_temp_at(dir, |dir, tmp| { + dir.mkdir_at(tmp, 0o700)?; + // Whatever the umask, the owner must be able to create entries inside. + dir.chmod_at(tmp, 0o700, SymlinkBehavior::NoFollow) + .and_then(|()| dir.open_subdir(tmp, SymlinkBehavior::NoFollow)) + .and_then(|staging| { + // Another directory may have been moved to `tmp` since. + let stat = staging.metadata()?; + if stat.uid() == geteuid().as_raw() && stat.mode() & 0o777 == 0o700 { + Ok(staging) + } else { + Err(io::ErrorKind::AlreadyExists.into()) + } + }) + .inspect_err(|_| { + // Removes only an empty directory: the one made above, or at + // worst an empty one moved to `tmp` since. + let _ = dir.unlink_at(tmp, true); + }) + })?; + + let created = staging + .mknod_at(name, metadata.mode(), metadata.rdev()) + .and_then(|()| { + let (uid, gid) = (metadata.uid(), metadata.gid()); + let node = staging.metadata_at(name, SymlinkBehavior::NoFollow)?; + // Ownership is best effort for unprivileged callers. As for + // regular files, if it did not take, the node belongs to whoever + // ran mv, so setuid and setgid are dropped. The rest of the mode, + // which the umask reduced, is restored. + let owned = (node.uid(), node.gid()) == (uid, gid) + || staging + .chown_at(name, Some(uid), Some(gid), SymlinkBehavior::NoFollow) + .is_ok(); + let mode = if owned { 0o7777 } else { 0o1777 }; + staging.chmod_at(name, metadata.mode() & mode, SymlinkBehavior::NoFollow) + }) + .and_then(|()| Ok(renameat(&staging, name, dir, name)?)) + .inspect_err(|_| { + let _ = staging.unlink_at(name, false); + }); + let _ = dir.unlink_at(&staging_name, true); + created +} + +/// Without `safe_traversal`, the node is created by path after removing the +/// destination, as `uucore::fs::replace_link` does on Redox. +#[cfg(target_os = "redox")] +fn copy_special_file(to: &Path, metadata: &fs::Metadata) -> io::Result<()> { + use nix::sys::stat::{Mode, SFlag, mknod}; + use std::os::unix::fs::MetadataExt; + + if to.symlink_metadata().is_ok() { fs::remove_file(to)?; } - // rustix::fs::mkfifoat is linux only - nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; - fs::remove_file(from) + let mode = metadata.mode() as nix::libc::mode_t; + mknod( + to, + SFlag::from_bits_truncate(mode & nix::libc::S_IFMT), + Mode::from_bits_truncate(mode), + metadata.rdev() as nix::libc::dev_t, + )?; + let (uid, gid) = (metadata.uid(), metadata.gid()); + let node = to.symlink_metadata()?; + // Setuid and setgid are dropped if the ownership did not take. + let owned = (node.uid(), node.gid()) == (uid, gid) + || unix::fs::lchown(to, Some(uid), Some(gid)).is_ok(); + let mode = if owned { 0o7777 } else { 0o1777 }; + fs::set_permissions(to, fs::Permissions::from_mode(metadata.mode() & mode)) } #[cfg(not(unix))] @@ -1018,7 +1124,7 @@ fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { clippy::unnecessary_wraps, reason = "fn sig must match on all platforms" )] -fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> { +fn rename_special_fallback(_from: &Path, _to: &Path, _metadata: &fs::Metadata) -> io::Result<()> { Ok(()) } @@ -1369,7 +1475,7 @@ fn copy_file_with_hardlinks_helper( // Copy a symlink file (no-follow). // rename_symlink_fallback already preserves ownership and removes the source. rename_symlink_fallback(from, to)?; - } else if is_fifo(from.symlink_metadata()?.file_type()) { + } else if from.symlink_metadata()?.file_type().is_fifo() { // rustix::fs::mkfifoat is linux only nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; // Preserve ownership (uid/gid) from the source diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index b2bf30e8732..77c12a62c14 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore mydir hardlinked tmpfs notty unwriteable myfolder SRCDATA DSTDATA REALDATA realfile -// spell-checker:ignore dirattr dirvalue setfattr getfattr +// spell-checker:ignore dirattr dirvalue setfattr getfattr Nofile use rstest::rstest; use std::io::Write; @@ -2973,6 +2973,256 @@ fn test_mv_cross_device_dir_refuses_symlink_at_recreated_dest() { assert_eq!(at.read("victim/guard"), "PROTECTED_DATA"); } +/// A cross-device move of a socket used to remove the destination and then +/// fail to copy the socket. Like GNU, the socket is recreated instead (#13145). +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_socket_replaces_dest() { + use std::os::unix::fs::FileTypeExt; + use std::os::unix::net::UnixListener; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + UnixListener::bind(at.plus("sock")).expect("bind socket"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + + scene.ucmd().arg("sock").arg(&dest).succeeds().no_output(); + + assert!(at.plus("sock").symlink_metadata().is_err()); + assert!(dest.symlink_metadata().unwrap().file_type().is_socket()); +} + +/// A fifo moved across devices replaces the destination and keeps its mode. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_fifo_replaces_dest_and_preserves_mode() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkfifo("fifo"); + set_permissions(at.plus("fifo"), Permissions::from_mode(0o604)).unwrap(); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + + scene.ucmd().arg("fifo").arg(&dest).succeeds().no_output(); + + let metadata = dest.symlink_metadata().unwrap(); + assert!(metadata.file_type().is_fifo()); + assert_eq!(metadata.permissions().mode() & 0o7777, 0o604); +} + +/// A destination reached through a symlinked directory is followed, as it is +/// for any other move. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_into_symlinked_parent() { + use std::os::unix::fs::{FileTypeExt, symlink}; + use std::os::unix::net::UnixListener; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + UnixListener::bind(at.plus("sock")).expect("bind socket"); + at.mkfifo("fifo"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let real = other_fs.path().join("real"); + let link = other_fs.path().join("link"); + std::fs::create_dir(&real).unwrap(); + symlink(&real, &link).unwrap(); + std::fs::write(real.join("sock_dest"), "old content").unwrap(); + + for (source, dest) in [("sock", "sock_dest"), ("fifo", "fifo_dest")] { + scene + .ucmd() + .arg(source) + .arg(link.join(dest)) + .succeeds() + .no_output(); + } + + let sock = real.join("sock_dest").symlink_metadata().unwrap(); + assert!(sock.file_type().is_socket()); + let fifo = real.join("fifo_dest").symlink_metadata().unwrap(); + assert!(fifo.file_type().is_fifo()); + assert!(link.symlink_metadata().unwrap().is_symlink()); +} + +/// Like a same-device move, a cross-device move of a special file only needs +/// write and search permission on the parent directories, not read. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_write_search_only_parents() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + if rustix::process::geteuid().is_root() { + return; + } + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("src"); + at.mkfifo("src/fifo"); + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dst = other_fs.path().join("dst"); + std::fs::create_dir(&dst).unwrap(); + std::fs::write(dst.join("fifo"), "old content").unwrap(); + set_permissions(at.plus("src"), Permissions::from_mode(0o300)).unwrap(); + set_permissions(&dst, Permissions::from_mode(0o300)).unwrap(); + + let result = scene.ucmd().arg("src/fifo").arg(dst.join("fifo")).run(); + + set_permissions(at.plus("src"), Permissions::from_mode(0o700)).unwrap(); + set_permissions(&dst, Permissions::from_mode(0o700)).unwrap(); + result.success().no_output(); + assert!(at.plus("src/fifo").symlink_metadata().is_err()); + let fifo = dst.join("fifo").symlink_metadata().unwrap(); + assert!(fifo.file_type().is_fifo()); +} + +/// The mode of a special file moved across devices is set before the file +/// appears at the destination name, so a file linked over that name in the +/// meantime keeps its own mode. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_mode_not_applied_to_swapped_entry() { + use std::fs::{Permissions, hard_link, remove_file, rename, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::thread; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let victim = other_fs.path().join("victim"); + std::fs::write(&victim, "").unwrap(); + set_permissions(&victim, Permissions::from_mode(0o600)).unwrap(); + let dest = other_fs.path().join("dest"); + + // Whenever a fifo shows up at `dest`, link `victim` over it. The link is + // made ahead of time, so that only a rename follows the check. + let done = Arc::new(AtomicBool::new(false)); + let swapper = thread::spawn({ + let (victim, dest, done) = (victim.clone(), dest.clone(), Arc::clone(&done)); + let link = other_fs.path().join("link"); + move || { + let mut linked = false; + while !done.load(Ordering::Relaxed) { + linked = linked || hard_link(&victim, &link).is_ok(); + if linked + && dest + .symlink_metadata() + .is_ok_and(|m| m.file_type().is_fifo()) + { + linked = rename(&link, &dest).is_err(); + } + } + } + }); + + for i in 0..1000 { + let fifo = format!("fifo{i}"); + at.mkfifo(&fifo); + set_permissions(at.plus(&fifo), Permissions::from_mode(0o646)).unwrap(); + scene.ucmd().arg(&fifo).arg(&dest).run(); + let _ = remove_file(&dest); + if victim.metadata().unwrap().permissions().mode() & 0o7777 != 0o600 { + break; + } + } + done.store(true, Ordering::Relaxed); + swapper.join().unwrap(); + + assert_eq!( + victim.metadata().unwrap().permissions().mode() & 0o7777, + 0o600, + "the fifo's mode was applied to a file linked over the destination" + ); +} + +/// Like GNU, a special file keeps its mode whatever the umask, including one +/// that takes the owner's write or search permission, which creating the +/// node in its private staging directory needs. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_under_owner_umask() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + + for umask in [0o100, 0o200, 0o400] { + let name = format!("fifo{umask:o}"); + at.mkfifo(&name); + set_permissions(at.plus(&name), Permissions::from_mode(0o646)).unwrap(); + let dest = other_fs.path().join(&name); + + scene + .ucmd() + .arg(&name) + .arg(&dest) + .umask(umask) + .succeeds() + .no_output(); + + let metadata = dest.symlink_metadata().unwrap(); + assert!(metadata.file_type().is_fifo()); + assert_eq!(metadata.permissions().mode() & 0o7777, 0o646); + } + assert_eq!(std::fs::read_dir(other_fs.path()).unwrap().count(), 3); +} + +/// The private directory a special file is staged in is removed again when +/// opening it fails, here because no descriptor is left for it. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_special_file_staging_removed_at_fd_limit() { + use rustix::process::Resource; + use std::os::unix::fs::FileTypeExt; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let other_fs = TempDir::new_in("/dev/shm/").expect("create temp dir in /dev/shm"); + let dest = other_fs.path().join("dest"); + std::fs::write(&dest, "old content").unwrap(); + at.mkfifo("fifo"); + + // Raise the limit one descriptor at a time: each run fails at a later + // step, one of them right after creating the staging directory, until + // the move succeeds. + for limit in 3..64 { + scene + .ucmd() + .arg("fifo") + .arg(&dest) + .limit(Resource::Nofile, limit, limit) + .run(); + + let entries: Vec<_> = std::fs::read_dir(other_fs.path()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .collect(); + assert_eq!(entries, ["dest"], "left behind at a limit of {limit}"); + if !at.plus("fifo").exists() { + break; + } + } + assert!(dest.symlink_metadata().unwrap().file_type().is_fifo()); +} + #[test] #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] fn test_mv_selinux_context() {