Race-conditions in GNU coreutils (wrt chown, chmod, setxattr system calls)

[Gaëtan LEURENT]

Hi,

GNU mv is subject to race-condition attack when it moves files across
filesystems. The problem is that it open() the new file, copies the
content, then close() it, and call chown(), chmod() and setxattr() on
the new path. This is unsafe because the path used to set the file
attributes could now point to another file than the one we have just
created.

== Here is an extract of "strace mv /test/a ." that shows the problem ==
rename("/test/a", "./a")                = -1 EXDEV (Invalid cross-device link)
unlink("./a")                           = -1 ENOENT (No such file or directory)
open("/test/a", O_RDONLY|O_LARGEFILE)   = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=7, ...}) = 0
open("./a", O_WRONLY|O_CREAT|O_LARGEFILE, 0100644) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
fstat64(3, {st_mode=S_IFREG|0644, st_size=7, ...}) = 0
read(3, "foobar\n", 4096)               = 7
write(4, "foobar\n", 7)                 = 7
read(3, "", 4096)                       = 0
close(4)                                = 0
close(3)                                = 0
utimes("./a", {1120662948, 589066})     = 0
chown32("./a", 1000, 1000)              = 0
getxattr("/test/a", "system.posix_acl_access", 0xbffff3c0, 132) = -1 EOPNOTSUPP 
(Operation not supported)
setxattr("./a", "system.posix_acl_access", 
"\x02\x00\x00\x00\x01\x00\x06\x00\xff\xff\xff\xff\x04\x00\x04\x00\xff\xff\xff\xff
 \x00\x04\x00\xff\xff\xff\xff", 28, ) = -1 EOPNOTSUPP (Operation not supported)
chmod("./a", 0100644)                   = 0
unlink("/test/a")                       = 0
=====

If some other user has write access to the directory where we are moving
the file, he could remove the new file and replace it by a hardlink to
some important file while we are copying the file contents content. Then
we could give him access to that important file. A typical attack
scenario would be when root is moving files between two directories that
belong to the same user and are not on the same filesystem, for instance
because one of the filesystems is full.

The correct way to set the file owner and the file access permissions is
to use fchown, fchmod and fgetxattr/fsetxattr on the filedescriptor
*before* closing the file.

== So the system call trace output should look like ==
rename("/test/a", "./a")                = -1 EXDEV (Invalid cross-device link)
unlink("./a")                           = -1 ENOENT (No such file or directory)
open("/test/a", O_RDONLY|O_LARGEFILE)   = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=7, ...}) = 0
open("./a", O_WRONLY|O_CREAT|O_LARGEFILE, 0100644) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
fstat64(3, {st_mode=S_IFREG|0644, st_size=7, ...}) = 0
read(3, "foobar\n", 4096)               = 7
write(4, "foobar\n", 7)                 = 7
read(3, "", 4096)                       = 0
fchown32(4, 1000, 1000)                  = 0
fchmod(4, 0100644)                       = 0
fgetxattr(3, "system.posix_acl_access", 0xbffff3c0, 132) = -1 EOPNOTSUPP 
(Operation not supported)
fsetxattr(4, "system.posix_acl_access", 
"\x02\x00\x00\x00\x01\x00\x06\x00\xff\xff\xff\xff\x04\x00\x04\x00\xff\xff\xff\xff
 \x00\x04\x00\xff\xff\xff\xff", 28, ) = -1 EOPNOTSUPP (Operation not supported)
close(4)                                = 0
close(3)                                = 0
utimes("./a", {1120662948, 589066})     = 0
unlink("/test/a")                       = 0
====

[Sorry for not sending a patch to better explain how to do, but the code
is a bit too hairy for me].

Some other coreutils programs do use chown/chmod/setxattr in a way that
does not seem safe to me:

- "cp -p" does the same as mv to set the new file attributes

- install also use chown, chmod, and setxattr after closing the file

- "mkdir -m" sets the directory access permissions with chmod after
  creating it, but this seems to be useless since it sets the umask to 0
  before, and gives the correct mode to the mkdir system call

- "mknod -m" does the same as "mkdir -m"

- "mkfifo -m" too

You should also check the other utilities in case I missed one.

Sincerely,

-- 
Gaëtan LEURENT