GNU bug report logs -
#12947
[brlink@debian.org: Bug#598018: install: temporary insecure file permissions]
Previous Next
Reported by: Samuel Bronson <naesten <at> gmail.com>
Date: Tue, 20 Nov 2012 19:07:01 UTC
Severity: normal
Tags: patch, security
Found in version 8.5
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your message dated Tue, 20 Nov 2012 13:20:45 -0800
with message-id <50ABF42D.50904 <at> cs.ucla.edu>
and subject line Re: bug#12947: [brlink <at> debian.org: Bug#598018: install: temporary insecure file permissions]
has caused the debbugs.gnu.org bug report #12947,
regarding [brlink <at> debian.org: Bug#598018: install: temporary insecure file permissions]
to be marked as done.
(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)
--
12947: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12947
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
[Message part 3 (text/plain, inline)]
Package: coreutils
Version: 8.5
Tags: security patch
From <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598018>:
[Message part 4 (message/rfc822, inline)]
[Message part 5 (text/plain, inline)]
Package: coreutils
Version: 8.5-1
Tags: security
X-Debbugs-CC: team <at> security.debian.org
Install a regular file with install creates the file with the same
permissions as the original file, copies the contents,
then changes the permissions of that file to 0600 and finally changes
ownerships and sets permissions to the ones requested with -m.
This means that if the target directory is more accessibly than the
original directory, or if the group will be set, the file can
for a short time be accessible to users it should not be accessible to.
Consider for example someone doing
install -m 750 -g shadow /etc/shadow /backup/shadow
results in:
stat64("/etc/shadow", {st_mode=S_IFREG|0640, st_size=778, ...}) = 0
lstat64("/backup/shadow", 0xffd932b4) = -1 ENOENT (No such file or directory)
open("/etc/shadow", O_RDONLY|O_LARGEFILE) = 3
fstat64(3, {st_mode=S_IFREG|0640, st_size=778, ...}) = 0
open("/backup/shadow", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE, 0640) = 4
fstat64(4, {st_mode=S_IFREG|0640, st_size=0, ...}) = 0
[...]
read(...)
write(...)
[...]
fchmod(4, 0600) = 0
close(4) = 0
close(3) = 0
lchown32("/backup/shadow", -1, 42) = 0
chmod("/backup/shadow", 0600) = 0
Which means the generated file will for a short time be readable by
accounts in group root (which should only be able to get the contests
if they also know the root password).
Other examples where this can be an issue are copying a file with mode
0644 in a directory only accessible to the current user to a directory
other people can access with install -m 600: again for a short time the
file will be accessible with mode 644.
The following patch fixes that (also attached to avoid transport problems):
diff -r -u -N a/src/copy.c b/src/copy.c
--- a/src/copy.c 2010-04-20 21:52:04.000000000 +0200
+++ b/src/copy.c 2010-09-25 13:44:01.000000000 +0200
@@ -2007,7 +2007,7 @@
used as the 3rd argument in the open call. Historical
practice passed all the source mode bits to 'open', but the extra
bits were ignored, so it should be the same either way. */
- if (! copy_reg (src_name, dst_name, x, src_mode & S_IRWXUGO,
+ if (! copy_reg (src_name, dst_name, x, dst_mode_bits & S_IRWXUGO,
omitted_permissions, &new_dst, &src_sb))
goto un_backup;
}
This patch should be safe as dst_mode_bits is src_mode unless set_mode
is set, which only install seems to set (and for install that behaviour
is always better).
Bernhard R. Link
[diff.diff (text/x-diff, attachment)]
[Message part 7 (text/plain, inline)]
I don't claim to understand it (copy_internal() is gigantic!), but it
seems like this should have been forwarded two years ago...
--
Hi! I'm a .signature virus! Copy me into your ~/.signature to help me spread!
[Message part 8 (message/rfc822, inline)]
Thanks, I installed this patch into the coreutils master branch,
and I'm marking the upstream coreutils bug as done.
From 7ee71d9ddad1435bbea00779bcd4c62482ea3473 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Tue, 20 Nov 2012 13:15:34 -0800
Subject: [PATCH] install: fix security race
* src/copy.c (copy_internal): Use DST_MODE_BITS, not SRC_MODE.
See Bernhard R. Link in <http://bugs.gnu.org/12947> and in
<http://bugs.debian.org/598018>.
---
src/copy.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 16aed03..7a35414 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2394,8 +2394,13 @@ copy_internal (char const *src_name, char const *dst_name,
/* POSIX says the permission bits of the source file must be
used as the 3rd argument in the open call. Historical
practice passed all the source mode bits to 'open', but the extra
- bits were ignored, so it should be the same either way. */
- if (! copy_reg (src_name, dst_name, x, src_mode & S_IRWXUGO,
+ bits were ignored, so it should be the same either way.
+
+ This call uses DST_MODE_BITS, not SRC_MODE. These are
+ normally the same, and the exception (where x->set_mode) is
+ used only by 'install', which POSIX does not specify and
+ where DST_MODE_BITS is what's wanted. */
+ if (! copy_reg (src_name, dst_name, x, dst_mode_bits & S_IRWXUGO,
omitted_permissions, &new_dst, &src_sb))
goto un_backup;
}
--
1.7.11.7
This bug report was last modified 12 years and 182 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.