* NEWS: Document fix for 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): Likewise.
* tests/cp/parent-perm-race: Test for the "cp --preserve=mode"
race fix in copy.c.
This commit is contained in:
Paul Eggert 2007-02-03 18:45:46 +01:00 committed by Jim Meyering
parent 1c73876f5f
commit b28a8851ed
5 changed files with 76 additions and 41 deletions

View File

@ -1,5 +1,14 @@
2007-02-02 Paul Eggert <eggert@cs.ucla.edu>
* NEWS: Document fix for 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): Likewise.
* tests/cp/parent-perm-race: Test for the "cp --preserve=mode"
race fix in copy.c.
* NEWS: Document fix for cp --parents.
* src/cp.c (make_dir_parents_private): Report the error sooner with
"cp --parents DIR/FILE DEST" when DIR is a non-directory, thus not

7
NEWS
View File

@ -13,6 +13,13 @@ GNU coreutils NEWS -*- outline -*-
"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.

View File

@ -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));

View File

@ -435,8 +435,16 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
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

View File

@ -1,7 +1,7 @@
#!/bin/sh
# Make sure cp -pR --parents isn't too generous with parent permissions.
# Copyright (C) 2006 Free Software Foundation, Inc.
# Copyright (C) 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
@ -32,37 +32,47 @@ framework_failure=0
mkdir -p $tmp || framework_failure=1
cd $tmp || framework_failure=1
umask 022
mkdir -p a d || framework_failure=1
mkfifo a/fifo || {
echo "$0: fifos not supported; skipping this test." 1>&2
(exit 77); exit 77
}
umask 002
mkdir mode ownership d || framework_failure=1
chmod g+s d 2>/dev/null # The cp test is valid either way.
# Copy a fifo's contents. That way, we can examine d/a's
# state while cp is running.
cp -p -R --copy-contents --parents a d &
cp_pid=$!
fail=0
(
# Now 'cp' is reading the fifo.
# Check the permissions of the temporary destination
# directory that 'cp' has made.
ls -ld d/a >d/ls.out
for attr in mode ownership
do
mkfifo $attr/fifo || {
echo "$0: fifos not supported; skipping this test." 1>&2
(exit 77); exit 77
}
# Close the fifo so that "cp" can continue. But output first,
# before exiting, otherwise some shells would optimize away the file
# descriptor that holds the fifo open.
echo foo
) >a/fifo
# Copy a fifo's contents. That way, we can examine d/$attr's
# state while cp is running.
cp --preserve=$attr -R --copy-contents --parents $attr d &
cp_pid=$!
case `cat d/ls.out` in
d???--[-S]--[-S]*)
fail=0;;
*)
fail=1;;
esac
(
# Now 'cp' is reading the fifo.
# Check the permissions of the temporary destination
# directory that 'cp' has made.
ls -ld d/$attr >d/$attr.ls
wait $cp_pid || fail=1
# Close the fifo so that "cp" can continue. But output first,
# before exiting, otherwise some shells would optimize away the file
# descriptor that holds the fifo open.
echo foo
) >$attr/fifo
ls_output=`cat d/$attr.ls` || fail=1
case $attr,$ls_output in
ownership,d???--[-S]--[-S]* | \
mode,d????-??-?* | \
mode,d??[-x]?w[-x]?-[-x]* )
;;
*)
fail=1;;
esac
wait $cp_pid || fail=1
done
(exit $fail); exit $fail