CC-tea/coreutils
1
0
Fork 0
mirror of git://git.sv.gnu.org/coreutils.git synced 2026-04-12 06:57:33 +02:00
Code Issues Projects Releases Wiki Activity

cp: fix security context race

Browse Source 
This fixes an issue introduced in the fix for Bug#11100.
* NEWS: Mention this.
* src/copy.c (copy_reg): Fix obscure bug where open-without-CREAT
failed with ENOENT and we forget to call set_process_security_ctx
before calling open-with-CREAT. Also, don’t bother to unlink
DST_NAME if open failed with ENOENT; and if unlink fails with
ENOENT, don’t consider that to be an error (someone else could
have removed the file for us, and that’s OK).  Also, don’t worry
about move mode, since we use O_EXCL|O_CREAT and so won’t open
an existing file.
This commit is contained in:
Paul Eggert
2021-11-17 13:22:06 -08:00
parent 0f84467a34
commit d0f035fc64
2 changed files with 26 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
NEWS
View File

@@ -8,6 +8,11 @@ GNU coreutils NEWS -*- outline -*-
All files would be processed correctly, but the exit status was incorrect.
[bug introduced in coreutils-9.0]
If 'cp -Z A B' checks B's status and some other process then removes B,
cp no longer creates B with a too-generous SELinux security context
before adjusting it to the correct value.
[bug introduced in coreutils-8.17]
On macOS, 'cp A B' no longer miscopies when A is in an APFS file system
and B is in some other file system.
[bug introduced in coreutils-9.0]

50
src/copy.c
View File

@@ -1120,7 +1120,9 @@ infer_scantype (int fd, struct stat const *sb,
OMITTED_PERMISSIONS after copying as needed.
X provides many option settings.
Return true if successful.
*NEW_DST is as in copy_internal.
*NEW_DST is initially as in copy_internal.
If successful, set *NEW_DST to true if the destination file was created and
to false otherwise; if unsuccessful, perhaps set *NEW_DST to some value.
SRC_SB is the result of calling follow_fstatat on SRC_NAME. */
static bool
@@ -1185,8 +1187,8 @@ copy_reg (char const *src_name, char const *dst_name,
the existing context according to the system default for the dest.
Note we set the context here, _after_ the file is opened, lest the
new context disallow that. */
if ((x->set_security_context || x->preserve_security_context)
&& 0 <= dest_desc)
if (0 <= dest_desc
&& (x->set_security_context || x->preserve_security_context))
{
if (! set_file_security_ctx (dst_name, false, x))
{
@@ -1198,38 +1200,45 @@ copy_reg (char const *src_name, char const *dst_name,
}
}
if (dest_desc < 0 && x->unlink_dest_after_failed_open)
if (dest_desc < 0 && dest_errno != ENOENT
&& x->unlink_dest_after_failed_open)
{
if (unlink (dst_name) != 0)
if (unlink (dst_name) == 0)
{
if (x->verbose)
printf (_("removed %s\n"), quoteaf (dst_name));
}
else if (errno != ENOENT)
{
error (0, errno, _("cannot remove %s"), quoteaf (dst_name));
return_val = false;
goto close_src_desc;
}
if (x->verbose)
printf (_("removed %s\n"), quoteaf (dst_name));
/* Tell caller that the destination file was unlinked. */
*new_dst = true;
dest_errno = ENOENT;
}
if (dest_desc < 0 && dest_errno == ENOENT)
{
/* Ensure there is no race where a file may be left without
an appropriate security context. */
if (x->set_security_context)
{
if (! set_process_security_ctx (src_name, dst_name, dst_mode,
*new_dst, x))
true, x))
{
return_val = false;
goto close_src_desc;
}
}
/* Tell caller that the destination file is created. */
*new_dst = true;
}
}
if (*new_dst)
{
open_with_O_CREAT:;
int open_flags = O_WRONLY | O_CREAT | O_BINARY;
dest_desc = open (dst_name, open_flags | O_EXCL,
dst_mode & ~omitted_permissions);
@@ -1280,23 +1289,6 @@ copy_reg (char const *src_name, char const *dst_name,
if (dest_desc < 0)
{
/* If we've just failed due to ENOENT for an ostensibly preexisting
destination (*new_dst was 0), that's a bit of a contradiction/race:
the prior stat/lstat said the file existed (*new_dst was 0), yet
the subsequent open-existing-file failed with ENOENT. With NFS,
the race window is wider still, since its meta-data caching tends
to make the stat succeed for a just-removed remote file, while the
more-definitive initial open call will fail with ENOENT. When this
situation arises, we attempt to open again, but this time with
O_CREAT. Do this only when not in move-mode, since when handling
a cross-device move, we must never open an existing destination. */
if (dest_errno == ENOENT && ! *new_dst && ! x->move_mode)
{
*new_dst = 1;
goto open_with_O_CREAT;
}
/* Otherwise, it's an error. */
error (0, dest_errno, _("cannot create regular file %s"),
quoteaf (dst_name));
return_val = false;