From c5dc2b777cefb015232acba832b608d73c427050 Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Fri, 31 Jul 2026 21:51:17 +0200
Subject: [PATCH] mv: strip setuid/setgid when the cross-device copy cannot
 preserve ownership

When rename(2) returns EXDEV, mv copies the file and then re-applies the
source mode, including setuid and setgid. The chown back to the original
owner is best-effort and silently ignored, so an unprivileged user moving a
root-owned setuid file across a filesystem boundary ended up owning a file
that still carried the bits.

preserve_ownership now reports whether the destination ended up with the
source's uid and gid, and the single-file fallback masks 0o6000 out of the
applied mode when it did not. GNU mv does the same, and uutils cp already
does (cp.rs, 'GNU cp strips setuid (04000) and setgid (02000) when...').

Measured in a user namespace with two tmpfs mounts, unprivileged uid moving
root-owned files:

  before: f6755 -> mode=6755, f4755 -> 4755, f2755 -> 2755 (owner mover)
  after : f6755 -> mode=755,  f4755 -> 755,  f2755 -> 755
  GNU   : f6755 -> mode=755,  f4755 -> 755,  f2755 -> 755

When ownership is preserved the bits survive, matching GNU. The directory
recursion path already matched GNU and is unchanged.

Reported by zerodaybugs via GHSA-6c4j-6pgg-xgg8.
---
 src/uu/mv/src/mv.rs      | 31 +++++++++++++++++++++++++------
 tests/by-util/test_mv.rs | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs
index 2c3202fbf9..0e2aa73c56 100644
--- a/src/uu/mv/src/mv.rs
+++ b/src/uu/mv/src/mv.rs
@@ -1324,8 +1324,18 @@ fn rename_file_fallback(
 
         // 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));
+        //
+        // If the chown did not take, the destination belongs to whoever ran
+        // `mv`, and re-applying setuid/setgid would hand them a binary running
+        // as themselves that used to run as someone else. GNU strips the bits
+        // in that case and so do we.
+        let ownership_preserved = preserve_ownership(from, to).unwrap_or(false);
+        let dest_mode = if ownership_preserved {
+            src_mode
+        } else {
+            src_mode & !0o6000
+        };
+        let _ = dst_file.set_permissions(Permissions::from_mode(dest_mode));
     }
 
     #[cfg(not(unix))]
@@ -1342,8 +1352,13 @@ fn rename_file_fallback(
 /// Preserve ownership (uid/gid) from source to destination.
 /// Uses lchown so it works on symlinks without following them.
 /// Errors are silently ignored for non-root users who cannot chown.
+///
+/// Returns whether the destination ended up with the source's uid and gid.
+/// Callers that re-apply the source mode must strip setuid/setgid when this is
+/// false: those bits on a file that changed hands grant the new owner's
+/// identity, not the original one's, which is why GNU drops them.
 #[cfg(unix)]
-fn preserve_ownership(from: &Path, to: &Path) -> io::Result<()> {
+fn preserve_ownership(from: &Path, to: &Path) -> io::Result<bool> {
     use std::os::unix::fs::MetadataExt;
 
     let source_meta = from.symlink_metadata()?;
@@ -1360,7 +1375,7 @@ fn preserve_ownership(from: &Path, to: &Path) -> io::Result<()> {
         // Use follow=false so lchown is used (works on symlinks)
         // Silently ignore errors: non-root users typically cannot chown to
         // arbitrary uid, matching GNU mv behavior which also uses best-effort.
-        let _ = wrap_chown(
+        if wrap_chown(
             to,
             &dest_meta,
             Some(uid),
@@ -1370,10 +1385,14 @@ fn preserve_ownership(from: &Path, to: &Path) -> io::Result<()> {
                 groups_only: false,
                 level: VerbosityLevel::Silent,
             },
-        );
+        )
+        .is_err()
+        {
+            return Ok(false);
+        }
     }
 
-    Ok(())
+    Ok(true)
 }
 
 fn is_empty_dir(path: &Path) -> bool {
diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs
index cbbe737922..a546eaa02d 100644
--- a/tests/by-util/test_mv.rs
+++ b/tests/by-util/test_mv.rs
@@ -1985,6 +1985,40 @@ mod inter_partition_copying {
     use uutests::util::TestScenario;
     use uutests::util_name;
 
+    // setuid/setgid must survive a cross-device move when ownership is
+    // preserved. The mover owns the file here, so the chown is a no-op and the
+    // bits are legitimate; only a failed chown justifies stripping them, and
+    // that needs a second uid, so it is exercised out of band rather than here.
+    #[test]
+    pub(crate) fn test_mv_inter_partition_keeps_setuid_when_ownership_preserved() {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        at.write("src", "src contents");
+        set_permissions(at.plus("src"), PermissionsExt::from_mode(0o6755))
+            .expect("Unable to set setuid/setgid on src");
+
+        let other_fs_tempdir =
+            TempDir::new_in("/dev/shm/").expect("Unable to create temp directory");
+        let dest = other_fs_tempdir.path().join("dest");
+
+        scene
+            .ucmd()
+            .arg("src")
+            .arg(dest.to_str().unwrap())
+            .succeeds();
+
+        let mode = fs::metadata(&dest)
+            .expect("destination should exist")
+            .permissions()
+            .mode()
+            & 0o7777;
+        assert_eq!(
+            mode, 0o6755,
+            "setuid/setgid must be kept when ownership was preserved, got {mode:o}"
+        );
+    }
+
     // Ensure that the copying code used in an inter-partition move unlinks the destination symlink.
     #[test]
     pub(crate) fn test_mv_unlinks_dest_symlink() {