Never copy through a symlink that cp has just created.

* src/copy.c (copy_internal): When same-file detection requires
'stat'ing the destination file, also 'lstat' it and ensure that
it wasn't the destination of a preceding copy operation.
This bug was introduced on 2007-06-18.
* tests/cp/abuse: New test for the above.
* tests/cp/Makefile.am (TESTS): Add abuse.

ChangeLog

@@ -1,3 +1,13 @@
+2007-10-02  Jim Meyering  <jim@meyering.net>
+
+	Never copy through a symlink that cp has just created.
+	* src/copy.c (copy_internal): When same-file detection requires
+	'stat'ing the destination file, also 'lstat' it and ensure that
+	it wasn't the destination of a preceding copy operation.
+	This bug was introduced on 2007-06-18.
+	* tests/cp/abuse: New test for the above.
+	* tests/cp/Makefile.am (TESTS): Add abuse.
+
 2007-09-30  Jim Meyering  <jim@meyering.net>
 
 	cp: do not abbreviate in --help output.

src/copy.c

@@ -1008,6 +1008,7 @@ copy_internal (char const *src_name, char const *dst_name,
   bool delayed_ok;
   bool copied_as_regular = false;
   bool preserve_metadata;
+  bool use_stat = true;
 
   if (x->move_mode && rename_succeeded)
     *rename_succeeded = false;
@@ -1054,12 +1055,14 @@ copy_internal (char const *src_name, char const *dst_name,
 	 However, if we intend to unlink or remove the destination
 	 first, use lstat, since a copy won't actually be made to the
 	 destination in that case.  */
-      if ((((S_ISREG (src_mode)
-	     || (x->copy_as_regular
-		 && ! (S_ISDIR (src_mode) || S_ISLNK (src_mode))))
-	    && ! (x->move_mode || x->symbolic_link || x->hard_link
-		  || x->backup_type != no_backups
-		  || x->unlink_dest_before_opening))
+      use_stat =
+	((S_ISREG (src_mode)
+	  || (x->copy_as_regular
+	      && ! (S_ISDIR (src_mode) || S_ISLNK (src_mode))))
+	 && ! (x->move_mode || x->symbolic_link || x->hard_link
+	       || x->backup_type != no_backups
+	       || x->unlink_dest_before_opening));
+      if ((use_stat
 	   ? stat (dst_name, &dst_sb)
 	   : lstat (dst_name, &dst_sb))
 	  != 0)
@@ -1076,7 +1079,7 @@ copy_internal (char const *src_name, char const *dst_name,
 	}
       else
 	{ /* Here, we know that dst_name exists, at least to the point
-	     that it is stat'able or lstat'table.  */
+	     that it is stat'able or lstat'able.  */
 	  bool return_now;
 	  bool unlink_src;
 
@@ -1297,6 +1300,41 @@ copy_internal (char const *src_name, char const *dst_name,
 	}
     }
 
+  /* Ensure we don't try to copy through a symlink that was
+     created by a prior call to this function.  */
+  if (command_line_arg
+      && x->dest_info
+      && ! x->move_mode
+      && x->backup_type == no_backups)
+    {
+      bool lstat_ok = true;
+      struct stat tmp_buf;
+      struct stat *dst_lstat_sb;
+
+      /* If we called lstat above, good: use that data.
+	 Otherwise, call lstat here, in case dst_name is a symlink.  */
+      if ( ! use_stat)
+	dst_lstat_sb = &dst_sb;
+      else
+	{
+	  if (lstat (dst_name, &tmp_buf) == 0)
+	    dst_lstat_sb = &tmp_buf;
+	  else
+	    lstat_ok = false;
+	}
+
+      /* Never copy through a symlink we've just created.  */
+      if (lstat_ok
+	  && S_ISLNK (dst_lstat_sb->st_mode)
+	  && seen_file (x->dest_info, dst_name, dst_lstat_sb))
+	{
+	  error (0, 0,
+		 _("will not copy %s through just-created symlink %s"),
+		 quote_n (0, src_name), quote_n (1, dst_name));
+	  return false;
+	}
+    }
+
   /* If the source is a directory, we don't always create the destination
      directory.  So --verbose should not announce anything until we're
      sure we'll create a directory. */
@@ -1820,8 +1858,10 @@ copy_internal (char const *src_name, char const *dst_name,
       goto un_backup;
     }
 
-  if (command_line_arg)
+  if (command_line_arg && x->dest_info)
     {
+      /* Now that the destination file is very likely to exist,
+	 add its info to the set.  */
       struct stat sb;
       if (lstat (dst_name, &sb) == 0)
 	record_file (x->dest_info, dst_name, &sb);

tests/cp/Makefile.am

@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 TESTS = \
+  abuse \
   proc-zero-len \
   thru-dangling \
   cp-a-selinux \

tests/cp/abuse

@@ -0,0 +1,57 @@
+#!/bin/sh
+# ensure that cp does not write through a just-copied symlink
+
+# Copyright (C) 2007 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  cp --version
+fi
+
+. $srcdir/../envvar-check
+. $srcdir/../lang-default
+. $srcdir/../test-lib.sh
+
+mkdir a b c || framework_failure
+ln -s ../t a/1 || framework_failure
+echo payload > b/1 || framework_failure
+
+echo "cp: will not copy \`b/1' through just-created symlink \`c/1'" \
+  > exp || framework_failure
+
+# Check both cases: a dangling symlink, and one pointing to a writable file.
+
+fail=0
+for i in dangling-dest existing-dest; do
+  test $i = existing-dest && echo i > t
+  test $i = dangling-dest && rm -f t
+
+  cp -a a/1 b/1 c 2> out && fail=1
+
+  compare out exp || fail=1
+
+  # When the destination is a dangling symlink,
+  # ensure that cp does not create it.
+  test $i = dangling-dest \
+    && test -f t && fail=1
+
+  # When the destination symlink points to a writable file,
+  # ensure that cp does not change it.
+  test $i = existing-dest \
+    && case $(cat t) in i);; *) fail=1;; esac
+done
+
+(exit $fail); exit $fail