diff --git a/Cargo.lock b/Cargo.lock index ddbc3d42510..d8b2af7721b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3816,6 +3816,7 @@ dependencies = [ "indicatif", "libc", "rustc-hash", + "rustix", "tempfile", "thiserror 2.0.18", "uucore", diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index ee0bc3b6a54..4cb450deb21 100644 --- a/src/uu/mv/Cargo.toml +++ b/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"] diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b750428f7e5..f69df771437 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)] @@ -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 ... : " 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()) } diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 2cb8346394d..f45900eb5c2 100644 --- a/src/uucore/Cargo.toml +++ b/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"] diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 2b42edaa349..1217d58e381 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/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>(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>(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>(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>(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(); diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 33d54383620..21bd42fad51 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2595,12 +2595,53 @@ fn test_special_file_different_filesystem() { std::fs::remove_dir_all("/dev/shm/tmp").unwrap(); } -/// Test cross-device move with permission denied error -/// This test mimics the scenario from the GNU part-fail test where -/// a cross-device move fails due to permission errors when removing the target file +/// Cross-device mv must fail with EACCES when creating a *new* file in a +/// read-only destination directory. #[test] #[cfg(target_os = "linux")] fn test_mv_cross_device_permission_denied() { + use std::fs::set_permissions; + use std::os::unix::fs::PermissionsExt; + use tempfile::TempDir; + use uutests::util::TestScenario; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("k", "source content"); + + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); + + // Remove write permissions from the directory so creating a new file inside fails. + set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o555)) + .expect("Unable to set directory permissions"); + + let target_file_path = other_fs_tempdir.path().join("new_target"); + + let result = scene + .ucmd() + .arg("-f") + .arg("k") + .arg(target_file_path.to_str().unwrap()) + .fails(); + + let stderr = result.stderr_str(); + assert!(stderr.contains("Permission denied") || stderr.contains("permission denied")); + + set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o755)) + .expect("Unable to restore directory permissions"); +} + +/// Cross-device mv must fail when the existing destination cannot be +/// unlinked because its parent directory is read-only. Mirrors GNU's +/// tests/mv/part-fail: a silent in-place O_TRUNC over the existing inode +/// would diverge from rename(2) semantics (preserving the dst's mode/owner +/// and producing no diagnostic), so we report "unable to remove target" +/// instead. +#[test] +#[cfg(target_os = "linux")] +fn test_mv_cross_device_existing_target_in_readonly_dir() { use std::fs::{set_permissions, write}; use std::os::unix::fs::PermissionsExt; use tempfile::TempDir; @@ -2614,15 +2655,12 @@ fn test_mv_cross_device_permission_denied() { let other_fs_tempdir = TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm"); - let target_file_path = other_fs_tempdir.path().join("k"); - write(&target_file_path, "target content").expect("Unable to write target file"); + let target_file_path = other_fs_tempdir.path().join("existing_target"); + write(&target_file_path, "old content").expect("Unable to write target file"); - // Remove write permissions from the directory to cause permission denied set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o555)) .expect("Unable to set directory permissions"); - // Attempt to move file to the other filesystem - // This should fail with a permission denied error let result = scene .ucmd() .arg("-f") @@ -2630,13 +2668,19 @@ fn test_mv_cross_device_permission_denied() { .arg(target_file_path.to_str().unwrap()) .fails(); - // Check that it contains permission denied and references the file - // The exact format may vary but should contain these key elements let stderr = result.stderr_str(); - assert!(stderr.contains("Permission denied") || stderr.contains("permission denied")); + assert!( + stderr.contains("Permission denied") || stderr.contains("permission denied"), + "expected permission-denied diagnostic, got: {stderr}" + ); set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o755)) .expect("Unable to restore directory permissions"); + + // Source must remain (mv must not have removed it after a failed copy). + assert!(at.file_exists("k"), "source must remain when mv fails"); + let preserved = std::fs::read_to_string(&target_file_path).expect("read dst"); + assert_eq!(preserved, "old content", "destination must not be modified"); } #[test] @@ -2865,6 +2909,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 ae9dc28d807..97869789d76 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -269,6 +269,102 @@ if echo "$AVAILABLE_UTILS" | grep -q "cp"; then rm -f test_cp_src test_cp_dst fi +# mv cross-device (EXDEV) must use fd-based *xattr ops (issue #10014). +if echo "$AVAILABLE_UTILS" | grep -q "mv" && [ -d /dev/shm ]; then + # Need different filesystems for the EXDEV fallback to fire. + 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 cross-device xattr check: TMPDIR and /dev/shm are on the same filesystem; skipped" + else + shm_probe=$(mktemp -p /dev/shm mv_xattr_probe.XXXXXX) + if setfattr -n user.probe -v ok "$shm_probe" 2>/dev/null; then + rm -f "$shm_probe" + cross_src=$(mktemp -p "$TEMP_DIR" cross_src.XXXXXX) + cross_dst=$(mktemp -u -p /dev/shm cross_dst.XXXXXX) + echo "cross-device payload" > "$cross_src" + if setfattr -n user.tag -v pinned "$cross_src" 2>/dev/null; then + if [ "$USE_MULTICALL" -eq 1 ]; then + mv_cmd="$COREUTILS_BIN mv" + else + mv_cmd="$PROJECT_ROOT/target/${PROFILE}/mv" + fi + strace -f -e trace='%file,fgetxattr,fsetxattr,flistxattr,getxattr,setxattr,listxattr' \ + -o strace_mv_xattr.log \ + $mv_cmd "$cross_src" "$cross_dst" 2>/dev/null || true + + # Path-based xattr calls on src/dst basenames = the TOCTOU pattern. + cross_src_base=$(basename "$cross_src") + cross_dst_base=$(basename "$cross_dst") + if grep -qE "(listxattr|getxattr|setxattr)\([^,]*($cross_src_base|$cross_dst_base)" strace_mv_xattr.log; then + cat strace_mv_xattr.log + fail_immediately "mv cross-device must use fd-based xattr ops (issue #10014)" + fi + if grep -qE 'flistxattr|fgetxattr|fsetxattr' strace_mv_xattr.log; then + echo "OK: mv cross-device uses fd-based xattr syscalls" + else + echo "WARN: mv cross-device xattr check: no xattr syscalls observed (xattr may have been filtered - check filesystem support)" + fi + rm -f "$cross_dst" + else + echo "WARN: mv cross-device xattr check: TMPDIR does not support user xattrs; skipped" + fi + else + rm -f "$shm_probe" + echo "WARN: mv cross-device xattr check: /dev/shm does not support user xattrs; skipped" + fi + fi +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 ""