uutils / coreutils

Cross-platform Rust rewrite of the GNU coreutils
https://uutils.github.io/
MIT License
17.24k stars 1.24k forks source link

install: insecure mode handling #6435

Open mjguzik opened 1 month ago

mjguzik commented 1 month ago

Files get created with their original mode and only chmodded later. If say the source file is 644 and the install command was invoked with -m 700, then there is a time window where the file can be opened by others whey should not be able to do it. gnu coreutils install makes sure to start with a 0600 file first preventing the problem.

How to repro:

mkdir /tmp/srcdir /tmp/dstdir touch /tmp/srcdir/foo chmod 644 /tmp/srcdir/foo install -m 700 /tmp/srcdir/foo /tmp/dstdir/foo

strace on the gnu coreutils variant:

[snip] openat(AT_FDCWD, "/tmp/dstdir/foo", O_RDONLY|O_PATH|O_DIRECTORY) = -1 ENOENT (No such file or directory) newfstatat(AT_FDCWD, "/tmp/srcdir/foo", {st_mode=S_IFREG|0644, st_size=0, ...}, 0) = 0 newfstatat(AT_FDCWD, "/tmp/dstdir/foo", 0x7fff20d9df60, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/tmp/srcdir/foo", O_RDONLY) = 3 fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0 openat(AT_FDCWD, "/tmp/dstdir/foo", O_WRONLY|O_CREAT|O_EXCL, 0600) = 4

The file is now 0600, also note O_EXCL.

ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported) fstat(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0 fadvise64(3, 0, 0, POSIX_FADV_SEQUENTIAL) = 0 uname({sysname="Linux", nodename="f", ...}) = 0 copy_file_range(3, NULL, 4, NULL, 9223372035781033984, 0) = 0 mmap(NULL, 139264, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7649ceeca000 read(3, "", 131072) = 0 fsetxattr(4, "system.posix_acl_access", "\2\0\0\0\1\0\6\0\377\377\377\377\4\0\0\0\377\377\377\377 \0\0\0\377\377\377\377", 28, 0) = 0 close(4) = 0 close(3) = 0 munmap(0x7649ceeca000, 139264) = 0 fchmodat(AT_FDCWD, "/tmp/dstdir/foo", 0700) = 0 [snip]

In contrast strace of the rust variant:

statx(AT_FDCWD, "/tmp/dstdir/foo", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffd05339fd0) = -1 ENOENT (No such file or directory) statx(0, NULL, AT_STATX_SYNC_AS_STAT, STATX_ALL, NULL) = -1 EFAULT (Bad address) statx(AT_FDCWD, "/tmp/srcdir/foo", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=0, ...}) = 0 statx(AT_FDCWD, "/tmp/dstdir/foo", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffd0533a0b0) = -1 ENOENT (No such file or directory) statx(AT_FDCWD, "/tmp/dstdir/foo", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffd0533a0b0) = -1 ENOENT (No such file or directory) statx(AT_FDCWD, "/tmp/dstdir", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFDIR|0775, stx_size=4096, ...}) = 0 statx(AT_FDCWD, "/tmp/dstdir/foo", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffd05337c90) = -1 ENOENT (No such file or directory) unlink("/tmp/dstdir/foo") = -1 ENOENT (No such file or directory)

Side note but notice excessive statx calls.

openat(AT_FDCWD, "/tmp/srcdir/foo", O_RDONLY|O_CLOEXEC) = 3 statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=0, ...}) = 0 openat(AT_FDCWD, "/tmp/dstdir/foo", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0100644) = 4

The file is now 644, anyone with access to the directory can open it.

statx(4, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=0, ...}) = 0 fchmod(4, 0100644) = 0

What is this fchmod doing?

copy_file_range(-1, NULL, -1, NULL, 1, 0) = -1 EBADF (Bad file descriptor) copy_file_range(3, NULL, 4, NULL, 1073741824, 0) = 0 read(3, "", 8192) = 0 close(4) = 0 close(3) = 0 chmod("/tmp/dstdir/foo", 0700) = 0

This should be fchmod on the fd. Interestingly gnu install rolls with a path-based variant as well.

Looks like the trouble stems from std::fs::copy.