From 1b8c8befd96bb1ebf02eeade5029a81e1b6c4dc1 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 2 Oct 2007 11:10:22 +0200 Subject: [PATCH] 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 | 10 ++++++++ src/copy.c | 56 ++++++++++++++++++++++++++++++++++++------- tests/cp/Makefile.am | 1 + tests/cp/abuse | 57 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 8 deletions(-) create mode 100755 tests/cp/abuse diff --git a/ChangeLog b/ChangeLog index b024418fc6..afc67cb673 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2007-10-02 Jim Meyering + + 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 cp: do not abbreviate in --help output. diff --git a/src/copy.c b/src/copy.c index 331fef8716..1ba28ed5f1 100644 --- 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 074f5f6f64..83e2126de6 100644 --- a/tests/cp/Makefile.am +++ b/tests/cp/Makefile.am @@ -16,6 +16,7 @@ # along with this program. If not, see . TESTS = \ + abuse \ proc-zero-len \ thru-dangling \ cp-a-selinux \ diff --git a/tests/cp/abuse b/tests/cp/abuse new file mode 100755 index 0000000000..04b10f7f6d --- /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 . + +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 -- 2.47.2