cp: Added overwrite detection for existing symlinks

src/uu/cp/src/copydir.rs

@@ -213,14 +213,15 @@ where
     // `path.ends_with(".")` does not seem to work
     path.as_ref().display().to_string().ends_with("/.")
 }
-
+#[allow(clippy::too_many_arguments)]
 /// Copy a single entry during a directory traversal.
 fn copy_direntry(
     progress_bar: &Option<ProgressBar>,
     entry: Entry,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
     preserve_hard_links: bool,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
 ) -> CopyResult<()> {
     let Entry {
@@ -267,6 +268,7 @@ fn copy_direntry(
                 local_to_target.as_path(),
                 options,
                 symlinked_files,
+                copied_destinations,
                 copied_files,
                 false,
             ) {
@@ -295,6 +297,7 @@ fn copy_direntry(
                 local_to_target.as_path(),
                 options,
                 symlinked_files,
+                copied_destinations,
                 copied_files,
                 false,
             ) {
@@ -323,12 +326,14 @@ fn copy_direntry(
 ///
 /// Any errors encountered copying files in the tree will be logged but
 /// will not cause a short-circuit.
+#[allow(clippy::too_many_arguments)]
 pub(crate) fn copy_directory(
     progress_bar: &Option<ProgressBar>,
     root: &Path,
     target: &Path,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
@@ -344,6 +349,7 @@ pub(crate) fn copy_directory(
             target,
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             source_in_command_line,
         );
@@ -417,6 +423,7 @@ pub(crate) fn copy_directory(
                     options,
                     symlinked_files,
                     preserve_hard_links,
+                    copied_destinations,
                     copied_files,
                 )?;
             }

src/uu/cp/src/cp.rs

@@ -1220,12 +1220,14 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
                 target_type,
                 options,
                 &mut symlinked_files,
+                &copied_destinations,
                 &mut copied_files,
             ) {
                 show_error_if_needed(&error);
                 non_fatal_errors = true;
+            } else {
+                copied_destinations.insert(dest.clone());
             }
-            copied_destinations.insert(dest.clone());
         }
         seen_sources.insert(source);
     }
@@ -1271,14 +1273,15 @@ fn construct_dest_path(
         TargetType::File => target.to_path_buf(),
     })
 }
-
+#[allow(clippy::too_many_arguments)]
 fn copy_source(
     progress_bar: &Option<ProgressBar>,
     source: &Path,
     target: &Path,
     target_type: TargetType,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
 ) -> CopyResult<()> {
     let source_path = Path::new(&source);
@@ -1290,6 +1293,7 @@ fn copy_source(
             target,
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             true,
         )
@@ -1302,6 +1306,7 @@ fn copy_source(
             dest.as_path(),
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             true,
         );
@@ -1910,13 +1915,14 @@ fn calculate_dest_permissions(
 ///
 /// The original permissions of `source` will be copied to `dest`
 /// after a successful copy.
-#[allow(clippy::cognitive_complexity)]
+#[allow(clippy::cognitive_complexity, clippy::too_many_arguments)]
 fn copy_file(
     progress_bar: &Option<ProgressBar>,
     source: &Path,
     dest: &Path,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
@@ -1934,6 +1940,17 @@ fn copy_file(
                 dest.display()
             )));
         }
+        // Fail if cp tries to copy two sources of the same name into a single symlink
+        // Example: "cp file1 dir1/file1 tmp" where "tmp" is a directory containing a symlink "file1" pointing to a file named "foo".
+        // foo will contain the contents of "file1" and "dir1/file1" will not be copied over to "tmp/file1"
+        if copied_destinations.contains(dest) {
+            return Err(Error::Error(format!(
+                "will not copy '{}' through just-created symlink '{}'",
+                source.display(),
+                dest.display()
+            )));
+        }
+
         let copy_contents = options.dereference(source_in_command_line) || !source_is_symlink;
         if copy_contents
             && !dest.exists()

tests/by-util/test_cp.rs

@@ -2660,6 +2660,62 @@ fn test_copy_through_dangling_symlink_no_dereference() {
         .no_stdout();
 }
 
+#[test]
+fn test_cp_symlink_overwrite_detection() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.mkdir("good");
+    at.mkdir("tmp");
+    at.write("README", "file1");
+    at.write("good/README", "file2");
+
+    at.symlink_file("tmp/foo", "tmp/README");
+    at.touch("tmp/foo");
+
+    ts.ucmd()
+        .arg("README")
+        .arg("good/README")
+        .arg("tmp")
+        .fails()
+        .stderr_only(if cfg!(target_os = "windows") {
+            "cp: will not copy 'good/README' through just-created symlink 'tmp\\README'\n"
+        } else if cfg!(target_os = "macos") {
+            "cp: will not overwrite just-created 'tmp/README' with 'good/README'\n"
+        } else {
+            "cp: will not copy 'good/README' through just-created symlink 'tmp/README'\n"
+        });
+    let contents = at.read("tmp/foo");
+    // None of the files seem to be copied in macos
+    if cfg!(not(target_os = "macos")) {
+        assert_eq!(contents, "file1");
+    }
+}
+
+#[test]
+fn test_cp_dangling_symlink_inside_directory() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.mkdir("good");
+    at.mkdir("tmp");
+    at.write("README", "file1");
+    at.write("good/README", "file2");
+
+    at.symlink_file("foo", "tmp/README");
+
+    ts.ucmd()
+        .arg("README")
+        .arg("good/README")
+        .arg("tmp")
+        .fails()
+        .stderr_only( if cfg!(target_os="windows") {
+            "cp: not writing through dangling symlink 'tmp\\README'\ncp: not writing through dangling symlink 'tmp\\README'\n"
+        } else {
+            "cp: not writing through dangling symlink 'tmp/README'\ncp: not writing through dangling symlink 'tmp/README'\n"
+            } );
+}
+
 /// Test for copying a dangling symbolic link and its permissions.
 #[cfg(not(target_os = "freebsd"))] // FIXME: fix this test for FreeBSD
 #[test]