diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e1a1dc122..21adaabd9 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -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")] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 83eefd211..d69ca1a06 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.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 (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() {