cp: strip setuid/setgid when chown fails during -p (#9750)

When `cp -p` cannot chown the destination to the source's owner
(e.g. a non-root user copying a root-owned setuid file), GNU cp
strips the setuid and setgid bits from the applied mode so the
destination does not give the copying user elevated privileges via
the copy. uutils was unconditionally applying the source mode,
producing user-owned files with a live setuid bit.

Track `ownership_preserved` alongside the existing chown retry
logic and, in the subsequent `handle_preserve(mode, ...)` block,
mask off `0o6000` from the source's mode when ownership could not
be preserved. The sticky bit (01000) is kept, matching GNU.
This commit is contained in:
Sylvestre Ledru
2026-04-21 12:55:20 +02:00
committed by Dorian Péron
parent 7ba1bf857c
commit bd170a5d4b
2 changed files with 49 additions and 2 deletions
+26 -1
View File
@@ -1780,6 +1780,14 @@ pub(crate) fn copy_attributes(
attributes.mode
};
// Track whether `chown` to the source's uid succeeded. If it did not
// (typical case: non-root user copying a root-owned setuid file), the
// mode preservation below must strip setuid/setgid so the destination
// does not give the copying user elevated privileges via the copy.
// Matches GNU cp. See issue #9750.
#[cfg(unix)]
let ownership_preserved = std::cell::Cell::new(true);
// Ownership must be changed first to avoid interfering with mode change.
#[cfg(unix)]
handle_preserve(attributes.ownership, || -> CopyResult<()> {
@@ -1812,6 +1820,7 @@ pub(crate) fn copy_attributes(
// gnu compatibility: cp doesn't report an error if it fails to set the ownership,
// and will fall back to changing only the gid if possible.
if try_chown(Some(dest_uid)).is_err() {
ownership_preserved.set(false);
let _ = try_chown(None);
}
Ok(())
@@ -1824,7 +1833,23 @@ pub(crate) fn copy_attributes(
// do nothing, since every symbolic link has the same
// permissions.
if !dest.is_symlink() {
fs::set_permissions(dest, source_metadata.permissions())
#[cfg(unix)]
let source_perms = {
use std::os::unix::fs::PermissionsExt;
let mut perms = source_metadata.permissions();
if !ownership_preserved.get() {
// GNU cp strips setuid (04000) and setgid (02000) when
// ownership could not be preserved. Keep the sticky bit
// (01000) and all rwx bits.
let mode = perms.mode() & !0o6000;
perms.set_mode(mode);
}
perms
};
#[cfg(not(unix))]
let source_perms = source_metadata.permissions();
fs::set_permissions(dest, source_perms)
.map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
// FIXME: Implement this for windows as well
#[cfg(feature = "feat_acl")]
+23 -1
View File
@@ -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 (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs
// spell-checker:ignore (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs ELOOP
// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx
#[cfg(unix)]
use rstest::rstest;
@@ -7898,6 +7898,28 @@ fn test_cp_preserve_context_with_z_fails() {
.stderr_contains("cannot combine");
}
// Covers the happy path for issue #9750: when chown succeeds (src owner ==
// current user), `cp -p` preserves setuid/setgid. The failure-path behavior —
// stripping setuid/setgid when chown cannot preserve ownership — requires a
// multi-user setup (source owned by a different uid, cp run as non-root) and
// is exercised by GNU's test suite; documenting here as future coverage.
#[test]
#[cfg(unix)]
fn test_cp_preserve_setuid_when_chown_succeeds() {
let (at, mut ucmd) = at_and_ucmd!();
at.touch("src");
at.set_mode("src", 0o4755);
ucmd.arg("-p").arg("src").arg("dst").succeeds();
let mode = at.metadata("dst").mode() & 0o7777;
assert_eq!(
mode & 0o4000,
0o4000,
"setuid bit should be preserved when chown succeeds (got mode {mode:o})"
);
}
#[test]
#[cfg(all(unix, not(target_os = "macos")))]
fn test_cp_recursive_non_utf8_source() {