From d48a65096682fe8259c4709bfbf9efae8cb30db3 Mon Sep 17 00:00:00 2001
From: Jeffrey Finkelstein <jeffrey.finkelstein@protonmail.com>
Date: Sun, 23 Oct 2022 17:40:30 -0400
Subject: [PATCH] cp: restrict copy through dangling symlink with -f

Change `cp` to terminate with an error when attempting to copy through
a dangling symbolic link with the `--force` option. Before this
commit,

    touch src
    ln -s no-such-file dest
    cp -f src dest

would incorrectly replace `dest` with the contents of `src`. After
this commit, it correctly fails with the error message

    cp: not writing through dangling symlink 'dest'
---
 src/uu/cp/src/cp.rs      | 16 +++++++++++-----
 tests/by-util/test_cp.rs | 13 +++++++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs
index 3a1697039..b81a31e54 100644
--- a/src/uu/cp/src/cp.rs
+++ b/src/uu/cp/src/cp.rs
@@ -1264,10 +1264,6 @@ fn copy_file(
     symlinked_files: &mut HashSet<FileInformation>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
-    if file_or_link_exists(dest) {
-        handle_existing_dest(source, dest, options, source_in_command_line)?;
-    }
-
     // Fail if dest is a dangling symlink or a symlink this program created previously
     if dest.is_symlink() {
         if FileInformation::from_path(dest, false)
@@ -1281,7 +1277,13 @@ fn copy_file(
             )));
         }
         let copy_contents = options.dereference(source_in_command_line) || !source.is_symlink();
-        if copy_contents && !dest.exists() {
+        if copy_contents
+            && !dest.exists()
+            && !matches!(
+                options.overwrite,
+                OverwriteMode::Clobber(ClobberMode::RemoveDestination)
+            )
+        {
             return Err(Error::Error(format!(
                 "not writing through dangling symlink '{}'",
                 dest.display()
@@ -1289,6 +1291,10 @@ fn copy_file(
         }
     }
 
+    if file_or_link_exists(dest) {
+        handle_existing_dest(source, dest, options, source_in_command_line)?;
+    }
+
     if options.verbose {
         println!("{}", context_for(source, dest));
     }
diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs
index 8285905c5..cb0f38484 100644
--- a/tests/by-util/test_cp.rs
+++ b/tests/by-util/test_cp.rs
@@ -1827,6 +1827,19 @@ fn test_copy_through_dangling_symlink_no_dereference_2() {
         .stderr_only("cp: not writing through dangling symlink 'target'");
 }
 
+/// Test that copy through a dangling symbolic link fails, even with --force.
+#[test]
+#[cfg(not(windows))]
+fn test_copy_through_dangling_symlink_force() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("src");
+    at.symlink_file("no-such-file", "dest");
+    ucmd.args(&["--force", "src", "dest"])
+        .fails()
+        .stderr_only("cp: not writing through dangling symlink 'dest'");
+    assert!(!at.file_exists("dest"));
+}
+
 #[test]
 #[cfg(unix)]
 fn test_cp_archive_on_nonexistent_file() {