Subject: bug#12947: [[email-masked]: Bug#598018: install: temporary insecure file permissions]

From: Samuel Bronson <[email-masked]>
Date: Tue, 20 Nov 2012 14:05:07 -0500

Package: coreutils
Version: 8.5
Tags: security patch

>From <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598018>:

Subject: Bug#598018: install: temporary insecure file permissions
From: "Bernhard R. Link" <[email-masked]>
To: [email-masked]
Date: Sat, 25 Sep 2010 14:17:32 +0200

Package: coreutils
Version: 8.5-1
Tags: security
X-Debbugs-CC: [email-masked]

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 -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;
     }

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!