From ed620d9f12949456c17665a70ef566595867aa14 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 28 Jul 2026 22:27:38 +0200 Subject: [PATCH 1/2] cp: do not descend into a destination subdirectory that is a symlink copy_direntry tested the destination with exists(), which resolves symlinks, so a destination entry that was itself a symlink to a directory looked like an existing directory and was descended into, writing the source subtree through the link and out of the destination tree. GNU refuses this. Treat a symlink at the destination as the non-directory it is. A symlinked directory named as the target itself stays allowed. --- src/uu/cp/src/copydir.rs | 12 +++++++++-- tests/by-util/test_cp.rs | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index bef87e6aa90..cafb33fbaab 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -277,10 +277,18 @@ fn copy_direntry( entry_is_dir_no_follow }; + // `exists()` resolves symlinks, so a destination entry that is itself a + // symlink to a directory would look like an already-existing directory and + // be descended into -- writing the source subtree through the link and out + // of the destination tree. GNU refuses this ("cannot overwrite + // non-directory ... with directory"), so treat a symlink at the destination + // as the non-directory it is. + let dest_is_symlink = entry.local_to_target.is_symlink(); + // If the source is a directory and the destination does not // exist, ... - if source_is_dir && !entry.local_to_target.exists() { - return if entry.target_is_file { + if source_is_dir && (dest_is_symlink || !entry.local_to_target.exists()) { + return if entry.target_is_file || dest_is_symlink { Err(translate!("cp-error-cannot-overwrite-non-directory-with-directory").into()) } else { build_dir( diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index d37634602c7..ac9215016d5 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -6695,6 +6695,50 @@ fn test_cp_parents_symlink_permissions_file() { ); } +/// A destination subdirectory that is really a symlink must not be descended +/// into: doing so writes the source subtree through the link and out of the +/// destination tree. GNU refuses with "cannot overwrite non-directory ... with +/// directory". +#[test] +#[cfg(unix)] +fn test_cp_recursive_dest_subdir_symlink_not_followed() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("src/hooks"); + at.write("src/hooks/payload", "PAYLOAD"); + at.mkdir("dst"); + at.mkdir("outside"); + at.symlink_dir("../outside", "dst/hooks"); + + scene + .ucmd() + .args(&["-a", "src/.", "dst"]) + .fails() + .stderr_contains("cannot overwrite non-directory"); + + assert!( + !at.file_exists("outside/payload"), + "cp wrote through the destination symlink and escaped the target tree" + ); +} + +/// A symlinked directory named as the *target* is still a legitimate +/// destination -- only entries discovered inside the tree are refused. +#[test] +#[cfg(unix)] +fn test_cp_recursive_target_dir_symlink_still_allowed() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("srcdir"); + at.write("srcdir/f", "X"); + at.mkdir("real"); + at.symlink_dir("real", "dstlink"); + + scene.ucmd().args(&["-r", "srcdir", "dstlink/"]).succeeds(); + + assert!(at.file_exists("real/srcdir/f")); +} + /// Test the behavior of preserving permissions of parents when copying through /// a symlink when source is a dir. #[test] From 23ec5c20551448de3531d4a9563195684917b186 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 29 Jul 2026 08:36:12 +0200 Subject: [PATCH 2/2] Update spell-checker ignore list in test_cp.rs --- tests/by-util/test_cp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index ac9215016d5..ae272b697ae 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore (flags) reflink (fs) tmpfs (linux) filefrag rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile subvolume uufs xattrs ELOOP -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx dstlink #[cfg(unix)] use rstest::rstest; use uucore::display::Quotable;