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.

diff --git a/ChangeLog b/ChangeLog
index b024418..afc67cb 100644 (file)
--- a/ChangeLog
+++ b/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.

diff --git a/src/copy.c b/src/copy.c
index 331fef8..1ba28ed 100644 (file)
--- a/src/copy.c
+++ b/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);

diff --git a/tests/cp/Makefile.am b/tests/cp/Makefile.am
index 074f5f6..83e2126 100644 (file)
--- a/tests/cp/Makefile.am
+++ b/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 \

diff --git a/tests/cp/abuse b/tests/cp/abuse
new file mode 100755 (executable)
index 0000000..04b10f7
--- /dev/null
+++ b/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