diff --git a/Cargo.lock b/Cargo.lock index 2d44c5834..18d7ece3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3790,6 +3790,7 @@ dependencies = [ "libc", "nix", "rustc-hash", + "rustix", "tempfile", "thiserror 2.0.18", "uucore", diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index b98e55dfc..4a169af0f 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -42,6 +42,11 @@ windows-sys = { workspace = true, features = [ "Win32_Storage_FileSystem", ] } +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } +rustix = { workspace = true, features = ["fs"] } + + [features] selinux = ["uucore/selinux"] diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 8b93520d4..d78be6d8d 100644 --- a/src/uu/mv/src/mv.rs +++ b/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)] @@ -902,16 +903,94 @@ 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 => { + #[cfg(not(target_os = "redox"))] + create_symlink_replace(&path_symlink_points_to, to)?; + #[cfg(target_os = "redox")] + { + fs::remove_file(to)?; + unix::fs::symlink(&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); } - // 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", + )) +} + #[cfg(windows)] fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { let path_symlink_points_to = fs::read_link(from)?; diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 05eeb9063..c836d3c0e 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2842,6 +2842,87 @@ fn test_mv_xattr_enotsup_silent() { } } +/// Cross-device mv of a symlink onto an existing file must replace the +/// destination atomically, matching GNU. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_symlink_onto_existing() { + use std::fs; + use std::os::unix::fs::symlink; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + symlink("/etc/passwd", at.plus("src_link")).expect("Failed to create source symlink"); + + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); + let dst_path = other_fs_tempdir.path().join("dst_exists"); + fs::write(&dst_path, "placeholder").expect("Failed to write placeholder dst"); + + scene + .ucmd() + .arg(at.plus_as_string("src_link")) + .arg(dst_path.to_str().unwrap()) + .succeeds() + .no_stderr(); + + assert!( + dst_path.is_symlink(), + "dst_exists should now be a symlink after the cross-device move" + ); + assert_eq!( + fs::read_link(&dst_path).expect("read_link failed"), + Path::new("/etc/passwd"), + ); + assert!( + !at.symlink_exists("src_link"), + "source symlink should be gone" + ); +} + +/// Cross-device mv of a symlink onto an existing directory (`-T`) must +/// fail without destroying the directory or its contents. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_symlink_onto_existing_dir() { + use std::fs; + use std::os::unix::fs::symlink; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + symlink("/etc/passwd", at.plus("src_link")).expect("Failed to create source symlink"); + + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); + let dst_dir = other_fs_tempdir.path().join("dst_dir"); + fs::create_dir(&dst_dir).expect("Failed to create destination directory"); + fs::write(dst_dir.join("guard"), "preserved").expect("Failed to write guard file"); + + scene + .ucmd() + .arg("-T") + .arg(at.plus_as_string("src_link")) + .arg(dst_dir.to_str().unwrap()) + .fails(); + + assert!( + dst_dir.is_dir(), + "destination directory must still exist after failed mv" + ); + assert!( + dst_dir.join("guard").is_file(), + "destination directory contents must be untouched" + ); + assert!( + at.symlink_exists("src_link"), + "source symlink must not be removed when mv fails" + ); +} + /// Test that symlinks inside directories are preserved during cross-device moves /// (not expanded into full copies of their targets) #[test] diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index ae9dc28d8..1b01434d6 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -269,6 +269,55 @@ if echo "$AVAILABLE_UTILS" | grep -q "cp"; then rm -f test_cp_src test_cp_dst fi +# mv cross-device symlink replacement must use *at syscalls against a +# pinned parent fd (matches GNU's force_symlinkat) so a concurrent rename +# of the parent directory cannot redirect the temp-and-rename dance, and +# the temp name must come from /dev/urandom rather than a guessable +# pid+nanos pattern. +if echo "$AVAILABLE_UTILS" | grep -q "mv" && [ -d /dev/shm ]; then + temp_fs_id=$(stat -f -c %i "$TEMP_DIR" 2>/dev/null || echo "") + shm_fs_id=$(stat -f -c %i /dev/shm 2>/dev/null || echo "") + if [ -z "$temp_fs_id" ] || [ -z "$shm_fs_id" ] || [ "$temp_fs_id" = "$shm_fs_id" ]; then + echo "WARN: mv symlink-replace check: TMPDIR and /dev/shm are on the same filesystem; skipped" + else + sym_src=$(mktemp -u -p "$TEMP_DIR" sym_src.XXXXXX) + sym_dst=$(mktemp -u -p /dev/shm sym_dst.XXXXXX) + ln -s /nowhere "$sym_src" + # Pre-existing dest forces the EEXIST branch into create_symlink_replace. + ln -s /elsewhere "$sym_dst" + + if [ "$USE_MULTICALL" -eq 1 ]; then + mv_cmd="$COREUTILS_BIN mv" + else + mv_cmd="$PROJECT_ROOT/target/${PROFILE}/mv" + fi + strace -f -e trace=openat,symlink,symlinkat,rename,renameat,renameat2,unlink,unlinkat,read \ + -o strace_mv_symlink_replace.log \ + $mv_cmd "$sym_src" "$sym_dst" 2>/dev/null || true + + sym_dst_base=$(basename "$sym_dst") + + # Temp-and-rename must happen against a real parent dirfd, not AT_FDCWD. + if ! grep -qE 'symlinkat\("[^"]*", [0-9]+,' strace_mv_symlink_replace.log; then + cat strace_mv_symlink_replace.log + fail_immediately "mv symlink replace must use symlinkat with a parent dirfd (issue #10010 follow-up)" + fi + if ! grep -qE "renameat2?\([0-9]+, \"[^\"]+\", [0-9]+, \"$sym_dst_base\"" strace_mv_symlink_replace.log; then + cat strace_mv_symlink_replace.log + fail_immediately "mv symlink replace must use renameat with a parent dirfd to commit the dest (issue #10010 follow-up)" + fi + + # Random temp source must be /dev/urandom (rejecting the prior pid+nanos scheme). + if ! grep -q 'openat(AT_FDCWD, "/dev/urandom"' strace_mv_symlink_replace.log; then + cat strace_mv_symlink_replace.log + fail_immediately "mv symlink replace must seed the temp name from /dev/urandom" + fi + + echo "OK: mv symlink replace uses symlinkat+renameat with parent dirfd and unguessable temp name" + rm -f "$sym_dst" + fi +fi + echo "" echo "✓ Basic safe traversal verification completed" echo ""