Here's a patch to fix a couple of bugs in 'cp'. In re-reviewing this
patch I see that I could split it into two patches if you like, since
the bugs are separably fixable. But for now I thought I'd just get
this out the door.
2007-02-02 Paul Eggert <[EMAIL PROTECTED]>
* NEWS: Document fixes to cp --parents and cp --preserve=mode.
* src/copy.c (copy_internal): Omit the group- or other-writeable
permissions when creating a directory, to avoid a race condition
if the special mode bits aren't right just after the directory is
created.
* src/cp.c (make_dir_parents_private): Report an error with "cp
--parents DIR/FILE DEST" and DIR turns out to be a non-directory.
* tests/cp/cp-parents: Test for the non-race-condition bug fixed
by the above change.
diff --git a/NEWS b/NEWS
index ad0fd1d..bd307c1 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,16 @@ GNU coreutils NEWS -*-
outline -*-
chmod no longer fails in an environment (e.g., a chroot) with openat
support but with insufficient /proc support.
+ "cp --parents F/G D" no longer creates a directory D/F when F is not
+ a directory (and F/G is therefore invalid).
+
+ "cp --preserve=mode" would create directories that briefly had
+ too-generous permissions in some cases. For example, when copying a
+ directory with permissions 777 the destination directory might
+ temporarily be setgid on some file systems, which would allow other
+ users to create subfiles with the same group as the directory. Fix
+ similar problems with 'install' and 'mv'.
+
cut no longer dumps core for usage like "cut -f2- f1 f2" with two or
more file arguments. This was due to a double-free bug, introduced
in coreutils-5.3.0.
diff --git a/src/copy.c b/src/copy.c
index d9ad254..5ec5a92 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1,5 +1,5 @@
/* copy.c -- core functions for copying files and directories
- Copyright (C) 89, 90, 91, 1995-2006 Free Software Foundation.
+ Copyright (C) 89, 90, 91, 1995-2007 Free Software Foundation.
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
@@ -1005,6 +1005,7 @@ copy_internal (char const *src_name, char const *dst_name,
struct stat dst_sb;
mode_t src_mode;
mode_t dst_mode IF_LINT (= 0);
+ mode_t dst_mode_bits;
mode_t omitted_permissions;
bool restore_dst_mode = false;
char *earlier_file = NULL;
@@ -1504,13 +1505,16 @@ copy_internal (char const *src_name, char const
*dst_name,
new_dst = true;
}
- /* If the ownership might change, omit some permissions at first, so
- unauthorized users cannot nip in before the file has the right
- ownership. */
+ /* If the ownership might change, or if it is a directory (whose
+ special mode bits may change after the directory is created),
+ omit some permissions at first, so unauthorized users cannot nip
+ in before the file is ready. */
+ dst_mode_bits = (x->set_mode ? x->mode : src_mode) & CHMOD_MODE_BITS;
omitted_permissions =
- (x->preserve_ownership
- ? (x->set_mode ? x->mode : src_mode) & (S_IRWXG | S_IRWXO)
- : 0);
+ (dst_mode_bits
+ & (x->preserve_ownership ? S_IRWXG | S_IRWXO
+ : S_ISDIR (src_mode) ? S_IWGRP | S_IWOTH
+ : 0));
delayed_ok = true;
@@ -1549,10 +1553,7 @@ copy_internal (char const *src_name, char const
*dst_name,
(src_mode & ~S_IRWXUGO) != 0. However, common practice is
to ask mkdir to copy all the CHMOD_MODE_BITS, letting mkdir
decide what to do with S_ISUID | S_ISGID | S_ISVTX. */
- mode_t mkdir_mode =
- ((x->set_mode ? x->mode : src_mode)
- & CHMOD_MODE_BITS & ~omitted_permissions);
- if (mkdir (dst_name, mkdir_mode) != 0)
+ if (mkdir (dst_name, dst_mode_bits & ~omitted_permissions) != 0)
{
error (0, errno, _("cannot create directory %s"),
quote (dst_name));
diff --git a/src/cp.c b/src/cp.c
index 8fe11a1..5759e0d 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1,5 +1,5 @@
/* cp.c -- file copying (main routines)
- Copyright (C) 89, 90, 91, 1995-2006 Free Software Foundation.
+ Copyright (C) 89, 90, 91, 1995-2007 Free Software Foundation.
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
@@ -415,6 +415,7 @@ make_dir_parents_private (char const *const_dir, size_t
src_offset,
mode_t src_mode;
mode_t omitted_permissions;
mode_t mkdir_mode;
+ int src_errno;
/* This component does not exist. We must set
*new_dst and new->mode inside this loop because,
@@ -422,15 +423,28 @@ make_dir_parents_private (char const *const_dir, size_t
src_offset,
make_dir_parents_private creates only e_dir/../a if
./b already exists. */
*new_dst = true;
- if (XSTAT (x, src, &stats))
+ src_errno = (XSTAT (x, src, &stats) != 0
+ ? errno
+ : S_ISDIR (stats.st_mode)
+ ? 0
+ : ENOTDIR);
+ if (src_errno)
{
- error (0, errno, _("failed to get attributes of %s"),
+ error (0, src_errno, _("failed to get attributes of %s"),
quote (src));
return false;
}
src_mode = stats.st_mode;
- omitted_permissions =
- x->preserve_ownership ? src_mode & (S_IRWXG | S_IRWXO) : 0;
+
+ /* If the ownership or special mode bits might change,
+ omit some permissions at first, so unauthorized users
+ cannot nip in before the file is ready. */
+ omitted_permissions = (src_mode
+ & (x->preserve_ownership
+ ? S_IRWXG | S_IRWXO
+ : x->preserve_mode
+ ? S_IWGRP | S_IWOTH
+ : 0));
/* POSIX says mkdir's behavior is implementation-defined when
(src_mode & ~S_IRWXUGO) != 0. However, common practice is
diff --git a/tests/cp/cp-parents b/tests/cp/cp-parents
index 373e607..6c123d2 100755
--- a/tests/cp/cp-parents
+++ b/tests/cp/cp-parents
@@ -2,7 +2,8 @@
# cp -R --parents dir-specified-with-trailing-slash/ other-dir
# would get a failed assertion.
-# Copyright (C) 2000, 2002, 2004, 2005, 2006 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2002, 2004, 2005, 2006, 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
@@ -48,6 +49,7 @@ cd $tmp || framework_failure=1
mkdir foo bar || framework_failure=1
mkdir -p a/b/c d e || framework_failure=1
+touch f || framework_failure=1
if test $framework_failure = 1; then
echo 'failure in testing framework'
@@ -65,6 +67,11 @@ cp -R --parents foo/ bar || fail=1
cp --verbose -a --parents a/b/c d > /dev/null 2>&1 || fail=1
test -d d/a/b/c || fail=1
+# With 6.7 and earlier, cp --parents f/g d would mistakenly create a
+# directory d/f, even though f is a regular file.
+cp --parents f/g d 2>/dev/null && fail=1
+test -d d/f && fail=1
+
# Check that re_protect works.
chmod go=w d/a
cp -a --parents d/a/b/c e || fail=1
M ChangeLog
M NEWS
M src/copy.c
M src/cp.c
M tests/cp/cp-parents
Committed as 048979f0a7f4e38078d9cc4f94e24a0da8ba1ae1
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
