mv: fix the remaining TOCTOU

src/uu/mv/Cargo.toml

@@ -30,6 +30,7 @@ uucore = { workspace = true, features = [
   "fs",
   "fsxattr",
   "perms",
+  "safe-copy",
   "update-control",
 ] }
 fluent = { workspace = true }
@@ -43,6 +44,7 @@ windows-sys = { workspace = true, features = [
 
 [target.'cfg(unix)'.dependencies]
 libc = { workspace = true }
+rustix = { workspace = true, features = ["fs"] }
 
 [features]
 selinux = ["uucore/selinux"]

src/uu/mv/src/mv.rs

@@ -4,6 +4,7 @@
 // file that was distributed with this source code.
 
 // spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized unwriteable
+// spell-checker:ignore renameat symlinkat unlinkat unguessability RDONLY CLOEXEC
 
 mod error;
 #[cfg(unix)]
@@ -903,16 +904,97 @@ fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> {
 #[cfg(unix)]
 fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let path_symlink_points_to = fs::read_link(from)?;
-    unix::fs::symlink(path_symlink_points_to, to)?;
+
+    // On AlreadyExists, fall through to atomic temp-and-rename so the
+    // destination is replaced rather than the call failing.
+    match unix::fs::symlink(&path_symlink_points_to, to) {
+        Ok(()) => {}
+        Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {
+            create_symlink_replace(&path_symlink_points_to, to)?;
+        }
+        Err(e) => return Err(e),
+    }
     #[cfg(not(any(target_os = "macos", target_os = "redox")))]
     {
-        let _ = copy_xattrs_if_supported(from, to);
+        let _ = fsxattr::copy_xattrs_ignore_unsupported(from, to);
     }
-    // Preserve ownership (uid/gid) from the source symlink
     let _ = preserve_ownership(from, to);
     fs::remove_file(from)
 }
 
+/// Create a symlink at `to`, atomically replacing any existing entry via
+/// a temp-name + `renameat(2)` so observers never see `to` missing.
+///
+/// Mirrors GNU's `force_symlinkat` in `force-link.c`: open the parent
+/// directory once and operate via `*at` syscalls so a concurrent rename
+/// of the parent cannot redirect the operation, and pick the temp name
+/// from `/dev/urandom` so it is unguessable to other users in that
+/// directory.
+#[cfg(all(unix, not(target_os = "redox")))]
+fn create_symlink_replace(target: &Path, to: &Path) -> io::Result<()> {
+    use io::Read;
+    use rustix::fs::{AtFlags, CWD, Mode, OFlags, openat, renameat, symlinkat, unlinkat};
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    // GNU's template is `CuXXXXXX`: a 2-char prefix plus 6 random chars
+    // drawn from a 62-char alphabet. Modulo bias on a 256→62 mapping is
+    // ~3% per slot — irrelevant for an 8-char unguessability budget.
+    const ALPHABET: &[u8; 62] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
+
+    let parent = to
+        .parent()
+        .filter(|p| !p.as_os_str().is_empty())
+        .unwrap_or_else(|| Path::new("."));
+    let basename = to
+        .file_name()
+        .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "invalid destination path"))?;
+
+    let dir_fd = openat(
+        CWD,
+        parent,
+        OFlags::DIRECTORY | OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW,
+        Mode::empty(),
+    )?;
+
+    let mut urandom = fs::File::open("/dev/urandom")?;
+
+    for _ in 0..32 {
+        let mut tmp_bytes = *b"Cu------";
+        let mut raw = [0u8; 6];
+        urandom.read_exact(&mut raw)?;
+        for (slot, byte) in tmp_bytes[2..].iter_mut().zip(raw) {
+            *slot = ALPHABET[(byte as usize) % ALPHABET.len()];
+        }
+        let tmp = OsStr::from_bytes(&tmp_bytes);
+
+        match symlinkat(target, &dir_fd, tmp) {
+            Ok(()) => {
+                if let Err(e) = renameat(&dir_fd, tmp, &dir_fd, basename) {
+                    let _ = unlinkat(&dir_fd, tmp, AtFlags::empty());
+                    return Err(io::Error::from(e));
+                }
+                return Ok(());
+            }
+            Err(e) if e == rustix::io::Errno::EXIST => {}
+            Err(e) => return Err(io::Error::from(e)),
+        }
+    }
+    Err(io::Error::new(
+        io::ErrorKind::AlreadyExists,
+        "could not allocate a unique temp name in destination directory",
+    ))
+}
+
+// Redox lacks the rustix `*at` primitives used above; fall back to a
+// non-atomic remove + symlink. A concurrent observer may briefly see
+// `to` missing, but this is the best we can do on this target.
+#[cfg(target_os = "redox")]
+fn create_symlink_replace(target: &Path, to: &Path) -> io::Result<()> {
+    fs::remove_file(to)?;
+    unix::fs::symlink(target, to)
+}
+
 #[cfg(windows)]
 fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let path_symlink_points_to = fs::read_link(from)?;
@@ -1171,7 +1253,7 @@ fn copy_file_with_hardlinks_helper(
         // Copy xattrs, ignoring ENOTSUP errors (filesystem doesn't support xattrs)
         #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
         {
-            let _ = copy_xattrs_if_supported(from, to);
+            let _ = fsxattr::copy_xattrs_ignore_unsupported(from, to);
         }
         // Preserve ownership (uid/gid) from the source
         let _ = preserve_ownership(from, to);
@@ -1186,14 +1268,23 @@ fn rename_file_fallback(
     #[cfg(unix)] hardlink_tracker: Option<&mut HardlinkTracker>,
     #[cfg(unix)] hardlink_scanner: Option<&HardlinkGroupScanner>,
 ) -> io::Result<()> {
-    // Remove existing target file if it exists
+    // Unlink any existing destination first so a cross-device 'mv' acts as
+    // if it were really using rename(2): the destination becomes a fresh
+    // inode (new mode/owner) instead of having the source's contents
+    // truncated into the existing target. Matches GNU mv (copy.c). The
+    // subsequent O_NOFOLLOW open in create_dest_restrictive closes the
+    // unlink/create TOCTOU window by refusing an attacker-planted symlink.
     if to.is_symlink() {
+        // Wrap with an "inter-device move failed: ... unable to remove
+        // target" context (GNU's preferred diagnostic for this case).
         fs::remove_file(to).map_err(|err| {
             let inter_device_msg = translate!("mv-error-inter-device-move-failed", "from" => from.quote(), "to" => to.quote(), "err" => err);
             io::Error::new(err.kind(), inter_device_msg)
         })?;
     } else if to.exists() {
-        // For non-symlinks, just remove the file without special error handling
+        // Regular-file destination: bubble up the bare errno so the outer
+        // "cannot move ... : <errno>" context is the user-visible message,
+        // matching GNU's accepted alternate diagnostic.
         fs::remove_file(to)?;
     }
 
@@ -1214,20 +1305,41 @@ fn rename_file_fallback(
         }
     }
 
-    // Regular file copy
-    fs::copy(from, to)
-        .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
-
-    // Copy xattrs, ignoring ENOTSUP errors (filesystem doesn't support xattrs)
-    #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
+    // Open src/dst with O_NOFOLLOW and keep the fds alive across copy,
+    // chown, xattr, and chmod so a concurrent path-swap can't redirect any
+    // step to a different inode.
+    #[cfg(unix)]
     {
-        let _ = copy_xattrs_if_supported(from, to);
+        use std::fs::Permissions;
+        use std::os::unix::fs::{MetadataExt, PermissionsExt};
+        use uucore::safe_copy::{create_dest_restrictive, open_source};
+        let src_file = open_source(from, /* nofollow */ true)
+            .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
+        let src_mode = src_file
+            .metadata()
+            .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?
+            .mode()
+            & 0o7777;
+        let mut dst_file = create_dest_restrictive(to, /* nofollow */ true)
+            .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
+        io::copy(&mut &src_file, &mut dst_file)
+            .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
+
+        #[cfg(not(any(target_os = "macos", target_os = "redox")))]
+        {
+            let _ = fsxattr::copy_xattrs_fd_ignore_unsupported(&src_file, &dst_file);
+        }
+
+        // chown before chmod: chown(2) clears setuid/setgid for non-root,
+        // so the final mode must be applied last to preserve those bits.
+        let _ = preserve_ownership(from, to);
+        let _ = dst_file.set_permissions(Permissions::from_mode(src_mode));
     }
 
-    // Preserve ownership (uid/gid) from the source file
-    #[cfg(unix)]
+    #[cfg(not(unix))]
     {
-        let _ = preserve_ownership(from, to);
+        fs::copy(from, to)
+            .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
     }
 
     fs::remove_file(from)
@@ -1272,18 +1384,6 @@ fn preserve_ownership(from: &Path, to: &Path) -> io::Result<()> {
     Ok(())
 }
 
-/// Copy xattrs from source to destination, ignoring ENOTSUP/EOPNOTSUPP errors.
-/// These errors indicate the filesystem doesn't support extended attributes,
-/// which is acceptable when moving files across filesystems.
-#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
-fn copy_xattrs_if_supported(from: &Path, to: &Path) -> io::Result<()> {
-    match fsxattr::copy_xattrs(from, to) {
-        Ok(()) => Ok(()),
-        Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => Ok(()),
-        Err(e) => Err(e),
-    }
-}
-
 fn is_empty_dir(path: &Path) -> bool {
     fs::read_dir(path).is_ok_and(|mut contents| contents.next().is_none())
 }

src/uucore/Cargo.toml

@@ -138,7 +138,7 @@ extendedbigdecimal = ["bigdecimal", "num-traits"]
 fast-inc = []
 fs = ["dunce", "libc", "winapi-util", "windows-sys"]
 fsext = ["libc", "windows-sys", "bstr"]
-fsxattr = ["xattr", "itertools"]
+fsxattr = ["xattr", "itertools", "libc"]
 hardware = []
 lines = []
 feat_systemd_logind = ["utmpx", "libc"]

src/uucore/src/lib/features/fsxattr.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore getxattr posix_acl_default posix_acl_access
+// spell-checker:ignore getxattr posix_acl_default posix_acl_access ENOTSUP EOPNOTSUPP renamer
 
 //! Set of functions to manage xattr on files and dirs
 use itertools::Itertools;
@@ -13,16 +13,24 @@ use std::ffi::{OsStr, OsString};
 use std::os::unix::ffi::OsStrExt;
 use std::path::Path;
 
-/// Copies extended attributes (xattrs) from one file or directory to another.
-///
-/// # Arguments
-///
-/// * `source` - A reference to the source path.
-/// * `dest` - A reference to the destination path.
-///
-/// # Returns
-///
-/// A result indicating success or failure.
+/// True if the error is `ENOTSUP` / `EOPNOTSUPP` (same errno on Linux,
+/// distinct on the BSDs).
+#[cfg(unix)]
+fn is_xattr_unsupported(err: &std::io::Error) -> bool {
+    matches!(
+        err.raw_os_error(),
+        Some(e) if e == libc::ENOTSUP || e == libc::EOPNOTSUPP
+    )
+}
+
+#[cfg(not(unix))]
+fn is_xattr_unsupported(_err: &std::io::Error) -> bool {
+    false
+}
+
+/// Copies extended attributes (xattrs) from one path to another.
+/// All errors propagate, including `ENOTSUP` / `EOPNOTSUPP`; for
+/// best-effort callers see [`copy_xattrs_ignore_unsupported`].
 pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
     for attr_name in xattr::list(&source)? {
         if let Some(value) = xattr::get(&source, &attr_name)? {
@@ -32,6 +40,43 @@ pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
     Ok(())
 }
 
+/// Like [`copy_xattrs`], but maps `ENOTSUP` / `EOPNOTSUPP` to `Ok(())`
+/// for callers where xattr preservation is best-effort.
+pub fn copy_xattrs_ignore_unsupported<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
+    match copy_xattrs(source, dest) {
+        Ok(()) => Ok(()),
+        Err(e) if is_xattr_unsupported(&e) => Ok(()),
+        Err(e) => Err(e),
+    }
+}
+
+/// Copies xattrs between two open file descriptors. Pins both inodes so
+/// list/get/set calls cannot be redirected by a concurrent renamer, unlike
+/// the path-based [`copy_xattrs`].
+#[cfg(unix)]
+pub fn copy_xattrs_fd(source: &std::fs::File, dest: &std::fs::File) -> std::io::Result<()> {
+    use xattr::FileExt;
+    for attr_name in source.list_xattr()? {
+        if let Some(value) = source.get_xattr(&attr_name)? {
+            dest.set_xattr(&attr_name, &value)?;
+        }
+    }
+    Ok(())
+}
+
+/// Like [`copy_xattrs_fd`], but maps `ENOTSUP` / `EOPNOTSUPP` to `Ok(())`.
+#[cfg(unix)]
+pub fn copy_xattrs_fd_ignore_unsupported(
+    source: &std::fs::File,
+    dest: &std::fs::File,
+) -> std::io::Result<()> {
+    match copy_xattrs_fd(source, dest) {
+        Ok(()) => Ok(()),
+        Err(e) if is_xattr_unsupported(&e) => Ok(()),
+        Err(e) => Err(e),
+    }
+}
+
 /// Like `copy_xattrs`, but skips the security.selinux attribute.
 #[cfg(unix)]
 pub fn copy_xattrs_skip_selinux<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
@@ -222,6 +267,35 @@ mod tests {
         assert_eq!(copied_value, test_value);
     }
 
+    #[test]
+    #[cfg(unix)]
+    fn test_copy_xattrs_fd() {
+        let temp_dir = tempdir().unwrap();
+        let source_path = temp_dir.path().join("source.txt");
+        let dest_path = temp_dir.path().join("dest.txt");
+
+        File::create(&source_path).unwrap();
+        File::create(&dest_path).unwrap();
+
+        let test_attr = "user.fd_test";
+        let test_value = b"fd value";
+        // Skip silently if the test fs doesn't support user xattrs.
+        if xattr::set(&source_path, test_attr, test_value).is_err() {
+            return;
+        }
+
+        let src = File::open(&source_path).unwrap();
+        let dst = std::fs::OpenOptions::new()
+            .write(true)
+            .open(&dest_path)
+            .unwrap();
+
+        copy_xattrs_fd(&src, &dst).unwrap();
+
+        let copied = xattr::get(&dest_path, test_attr).unwrap().unwrap();
+        assert_eq!(copied, test_value);
+    }
+
     #[test]
     fn test_apply_and_retrieve_xattrs() {
         let temp_dir = tempdir().unwrap();