Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: insecure mode handling #6435

Open
mjguzik opened this issue May 26, 2024 · 0 comments · May be fixed by #6595
Open

install: insecure mode handling #6435

mjguzik opened this issue May 26, 2024 · 0 comments · May be fixed by #6595

Comments

@mjguzik
Copy link

mjguzik commented May 26, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants