diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index bef87e6aa9..cafb33fbaa 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 d37634602c..ae272b697a 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; @@ -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]