From ebc1db4851f3fd10d677a9593827f53589fc2203 Mon Sep 17 00:00:00 2001 From: nagendramohan Date: Tue, 15 Sep 2026 14:09:16 +0530 Subject: [PATCH 1/2] cp: apply the umask to directories created by cp -r Without a preserve flag, cp -r gave every directory it created the source's mode unmasked, so a 777 source directory became 777 regardless of the umask, while regular files in the same copy were masked correctly. The final permission pass defaulted a freshly-created directory to preserving the source mode. Apply the umask in that defaulted case, as GNU does, while a real preserve (-p/-a) and --no-preserve=mode are unchanged. Fixes #14549 Signed-off-by: nagendramohan --- src/uu/cp/src/cp.rs | 8 ++++++++ tests/by-util/test_cp.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 321dc3c8b5..7736ce9fc6 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1892,6 +1892,11 @@ pub(crate) fn copy_attributes( attributes.mode }; + // A created directory only defaults to copying the source mode; unlike an + // explicit preserve (-p/-a), GNU applies the umask to it. + let apply_umask_to_mode = + dest_is_freshly_created_dir && !matches!(attributes.mode, Preserve::Yes { .. }); + // 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 @@ -1956,6 +1961,9 @@ pub(crate) fn copy_attributes( let mode = perms.mode() & !0o6000; perms.set_mode(mode); } + if apply_umask_to_mode { + perms.set_mode(perms.mode() & !uucore::mode::get_umask()); + } perms }; #[cfg(not(unix))] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 862d11eb5b..c40dbad558 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1056,6 +1056,22 @@ fn test_cp_umask_stripping_owner_write_bit_reflink_never() { } } +// Regression for #14549: `cp -r` (without preserve) must apply the umask to +// directories it creates, matching GNU, instead of copying the source's mode. +#[test] +#[cfg(unix)] +fn test_cp_recursive_dir_applies_umask() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("src"); + at.mkdir("src/dir"); + at.set_mode("src/dir", 0o777); + + ucmd.umask(0o077).args(&["-r", "src", "d"]).succeeds(); + + // 0o777 & ~0o077 = 0o700, not the source's raw 0o777. + assert_eq!(at.metadata("d/dir").permissions().mode() & 0o777, 0o700); +} + // When --reflink=always fails, GNU cp removes a destination it created // itself but keeps a pre-existing (truncated) one. Only observable on // filesystems without clone support; when the clone succeeds there is From fdc37e80e48463492d2d524910270d4a1a2adb56 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Thu, 17 Sep 2026 21:12:00 +0200 Subject: [PATCH 2/2] cp: clear setuid/setgid on directories created by cp -r --- src/uu/cp/src/cp.rs | 5 +++- tests/by-util/test_cp.rs | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 7736ce9fc6..046ac6a873 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1962,7 +1962,10 @@ pub(crate) fn copy_attributes( perms.set_mode(mode); } if apply_umask_to_mode { - perms.set_mode(perms.mode() & !uucore::mode::get_umask()); + // The umask never covers setuid/setgid, so clear them + // explicitly: a non-preserving copy must not carry the + // source's set-user/group-ID bits into the new directory. + perms.set_mode(perms.mode() & !0o6000 & !uucore::mode::get_umask()); } perms }; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index c40dbad558..a43cdc7216 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1072,6 +1072,60 @@ fn test_cp_recursive_dir_applies_umask() { assert_eq!(at.metadata("d/dir").permissions().mode() & 0o777, 0o700); } +// The umask alone never covers setuid/setgid, so a non-preserving `cp -r` +// must clear them on the directories it creates. The sticky bit survives. +#[test] +#[cfg(unix)] +fn test_cp_recursive_dir_drops_setuid_setgid() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("tree"); + for (name, mode) in [ + ("tree/setgid", 0o2731u32), + ("tree/setuid", 0o4713), + ("tree/sticky", 0o1735), + ] { + at.mkdir(name); + at.set_mode(name, mode); + } + + scene + .ucmd() + .umask(0o026) + .args(&["-r", "tree", "plain"]) + .succeeds(); + + assert_eq!( + at.metadata("plain/setgid").permissions().mode() & 0o7777, + 0o711 + ); + assert_eq!( + at.metadata("plain/setuid").permissions().mode() & 0o7777, + 0o711 + ); + assert_eq!( + at.metadata("plain/sticky").permissions().mode() & 0o7777, + 0o1711 + ); + + // An explicit preserve keeps the mode as-is, umask and special bits alike. + scene + .ucmd() + .umask(0o026) + .args(&["-r", "--preserve=mode", "tree", "kept"]) + .succeeds(); + + assert_eq!( + at.metadata("kept/setgid").permissions().mode() & 0o7777, + 0o2731 + ); + assert_eq!( + at.metadata("kept/setuid").permissions().mode() & 0o7777, + 0o4713 + ); +} + // When --reflink=always fails, GNU cp removes a destination it created // itself but keeps a pre-existing (truncated) one. Only observable on // filesystems without clone support; when the clone succeeds there is