git: 0729d1e8fd90 - main - cp: Never follow symbolic links in destination.

The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=0729d1e8fd90afc2f19e9b9834533181caf6ddee

commit 0729d1e8fd90afc2f19e9b9834533181caf6ddee
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-04-08 22:41:33 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-04-08 22:41:33 +0000

    cp: Never follow symbolic links in destination.
    
    Historically, BSD cp has followed symbolic links in the destination
    when copying recursively, while GNU cp has not.  POSIX is somewhat
    vague on the topic, but both interpretations are within bounds.  In
    33ad990ce974, cp was changed to apply the same logic for symbolic
    links in the destination as for symbolic links in the source: follow
    if not recursing (which is moot, as this situation can only arise
    while recursing) or if the `-L` option was given.  There is no support
    for this in POSIX.  We can either switch back, or go all the way.
    
    Having carefully weighed the kind of trouble you can run into by
    following unexpected symlinks up against the kind of trouble you can
    run into by not following symlinks you expected to follow, we choose
    to go all the way.
    
    Note that this means we need to stat the destination twice: once,
    following links, to check if it is or references the same file as the
    source, and a second time, not following links, to set the dne flag
    and determine the destination's type.
    
    While here, remove a needless complication in the dne logic.  We don't
    need to explicitly reject overwriting a directory with a non-directory,
    because it will fail anyway.
    
    Finally, add test cases for copying a directory to a symlink and
    overwriting a directory with a non-directory.
    
    MFC after:      never
    Relnotes:       yes
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D44578
---
 bin/cp/cp.c             | 52 +++++++++++++------------------------------------
 bin/cp/tests/cp_test.sh | 31 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/bin/cp/cp.c b/bin/cp/cp.c
index 02b879006a4f..cee412e57264 100644
--- a/bin/cp/cp.c
+++ b/bin/cp/cp.c
@@ -260,22 +260,6 @@ main(int argc, char *argv[])
 	    &to_stat)));
 }
 
-/* Does the right thing based on -R + -H/-L/-P */
-static int
-copy_stat(const char *path, struct stat *sb)
-{
-
-	/*
-	 * For -R -H/-P, we need to lstat() instead; copy() cares about the link
-	 * itself rather than the target if we're not following links during the
-	 * traversal.
-	 */
-	if (!Rflag || Lflag)
-		return (stat(path, sb));
-	return (lstat(path, sb));
-}
-
-
 static int
 copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 {
@@ -403,7 +387,6 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 					continue;
 				}
 
-
 				if (asprintf(&recurse_path, "%s/%s", to.p_path,
 				    rootname) == -1)
 					err(1, "asprintf");
@@ -452,30 +435,21 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			continue;
 		}
 
-		/* Not an error but need to remember it happened. */
-		if (copy_stat(to.p_path, &to_stat) == -1)
-			dne = 1;
-		else {
-			if (to_stat.st_dev == curr->fts_statp->st_dev &&
-			    to_stat.st_ino == curr->fts_statp->st_ino) {
-				warnx("%s and %s are identical (not copied).",
-				    to.p_path, curr->fts_path);
-				badcp = rval = 1;
-				if (S_ISDIR(curr->fts_statp->st_mode))
-					(void)fts_set(ftsp, curr, FTS_SKIP);
-				continue;
-			}
-			if (!S_ISDIR(curr->fts_statp->st_mode) &&
-			    S_ISDIR(to_stat.st_mode)) {
-				warnx("cannot overwrite directory %s with "
-				    "non-directory %s",
-				    to.p_path, curr->fts_path);
-				badcp = rval = 1;
-				continue;
-			}
-			dne = 0;
+		/* Check if source and destination are identical. */
+		if (stat(to.p_path, &to_stat) == 0 &&
+		    to_stat.st_dev == curr->fts_statp->st_dev &&
+		    to_stat.st_ino == curr->fts_statp->st_ino) {
+			warnx("%s and %s are identical (not copied).",
+			    to.p_path, curr->fts_path);
+			badcp = rval = 1;
+			if (S_ISDIR(curr->fts_statp->st_mode))
+				(void)fts_set(ftsp, curr, FTS_SKIP);
+			continue;
 		}
 
+		/* Not an error but need to remember it happened. */
+		dne = lstat(to.p_path, &to_stat) != 0;
+
 		switch (curr->fts_statp->st_mode & S_IFMT) {
 		case S_IFLNK:
 			/* Catch special case of a non-dangling symlink. */
diff --git a/bin/cp/tests/cp_test.sh b/bin/cp/tests/cp_test.sh
index 397c06d75bbb..5c581e06ab8e 100755
--- a/bin/cp/tests/cp_test.sh
+++ b/bin/cp/tests/cp_test.sh
@@ -363,6 +363,35 @@ symlink_exists_force_body()
 	atf_check -o inline:"foo\n" readlink bar
 }
 
+atf_test_case directory_to_symlink
+directory_to_symlink_body()
+{
+	mkdir -p foo
+	ln -s .. foo/bar
+	mkdir bar
+	touch bar/baz
+	atf_check -s not-exit:0 -e match:"Not a directory" \
+	    cp -R bar foo
+	atf_check -s not-exit:0 -e match:"Not a directory" \
+	    cp -r bar foo
+}
+
+atf_test_case overwrite_directory
+overwrite_directory_body()
+{
+	mkdir -p foo/bar/baz
+	touch bar
+	atf_check -s not-exit:0 -e match:"Is a directory" \
+	    cp bar foo
+	rm bar
+	mkdir bar
+	touch bar/baz
+	atf_check -s not-exit:0 -e match:"Is a directory" \
+	    cp -R bar foo
+	atf_check -s not-exit:0 -e match:"Is a directory" \
+	    cp -r bar foo
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case basic
@@ -388,4 +417,6 @@ atf_init_test_cases()
 	atf_add_test_case symlink
 	atf_add_test_case symlink_exists
 	atf_add_test_case symlink_exists_force
+	atf_add_test_case directory_to_symlink
+	atf_add_test_case overwrite_directory
 }