francma / wob

A lightweight overlay volume/backlight/progress/anything bar for Wayland.
ISC License
911 stars 50 forks source link

Crash with SIGSYS due to illegal newfstatat system call #62

Closed kris7t closed 3 years ago

kris7t commented 3 years ago

Starting with glibc 2.33, wob crashes due to a SIGSYS signal immediately after reading the first line of input:

Core was generated by `./wob'.
Program terminated with signal SIGSYS, Bad system call.
#0  0x00007f569a27a67e in fstatat64 () from /usr/lib/libc.so.6
(gdb) ba
#0  0x00007f569a27a67e in fstatat64 () from /usr/lib/libc.so.6
#1  0x00007f569a1ff763 in _IO_file_doallocate () from /usr/lib/libc.so.6
#2  0x00007f569a20e090 in _IO_doallocbuf () from /usr/lib/libc.so.6
#3  0x00007f569a20cfcc in __GI__IO_file_underflow () from /usr/lib/libc.so.6
#4  0x00007f569a20e146 in _IO_default_uflow () from /usr/lib/libc.so.6
#5  0x00007f569a20116c in _IO_getline_info () from /usr/lib/libc.so.6
#6  0x00007f569a2000ba in fgets () from /usr/lib/libc.so.6
#7  0x0000556e1203fccb in main (argc=1, argv=0x7ffdff3b0de8) at ../main.c:783

A small test program without Seccomp confirms that the newfstatat system call is now used by fgets

#include <stdio.h>

int main(void) {
    char s[100];
    return !fgets(s, sizeof(s), stdin);
}

giving rise to the following system call trace:

$ echo | strace ./a.out
execve("./a.out", ["./a.out"], 0x7ffd9d91ddd0 /* 74 vars */) = 0
brk(NULL)                               = 0x5587ce579000
arch_prctl(0x3001 /* ARCH_??? */, 0x7ffe34ad6210) = -1 EINVAL (Invalid argument)
access("/etc/ld.so.preload", R_OK)      = 0
openat(AT_FDCWD, "/etc/ld.so.preload", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=0, ...}, AT_EMPTY_PATH) = 0
close(3)                                = 0
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=239432, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 239432, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f9820752000
close(3)                                = 0
openat(AT_FDCWD, "/usr/lib/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0`|\2\0\0\0\0\0"..., 832) = 832
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
pread64(3, "\4\0\0\0 \0\0\0\5\0\0\0GNU\0\2\0\0\300\4\0\0\0\3\0\0\0\0\0\0\0"..., 48, 848) = 48
pread64(3, "\4\0\0\0\24\0\0\0\3\0\0\0GNU\0\253\363\262\251\201\\\f\326\344(\f\331\224t\323A"..., 68, 896) = 68
newfstatat(3, "", {st_mode=S_IFREG|0755, st_size=2155984, ...}, AT_EMPTY_PATH) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f9820750000
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 784, 64) = 784
mmap(NULL, 1884632, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f9820583000
mmap(0x7f98205a9000, 1359872, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f98205a9000
mmap(0x7f98206f5000, 311296, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x172000) = 0x7f98206f5000
mmap(0x7f9820741000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1bd000) = 0x7f9820741000
mmap(0x7f9820747000, 33240, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f9820747000
close(3)                                = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f9820581000
arch_prctl(ARCH_SET_FS, 0x7f9820751580) = 0
mprotect(0x7f9820741000, 12288, PROT_READ) = 0
mprotect(0x5587cd3ea000, 4096, PROT_READ) = 0
mprotect(0x7f98207bc000, 8192, PROT_READ) = 0
munmap(0x7f9820752000, 239432)          = 0
newfstatat(0, "", {st_mode=S_IFIFO|0600, st_size=0, ...}, AT_EMPTY_PATH) = 0
brk(NULL)                               = 0x5587ce579000
brk(0x5587ce59a000)                     = 0x5587ce59a000
read(0, "\n", 4096)                     = 1
exit_group(0)                           = ?
+++ exited with 0 +++
francma commented 3 years ago

Thank you for pointing this out. I decided to roll my own fix because I don't like this part of int fstatat(int dirfd, const char *pathname, struct stat *buf, int flags); syscall:

If pathname is absolute, then dirfd is ignored.

Allowing fstating anything on filesystem.

I also tried restricting fstatat with seccomp rule like this:

rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(read), 3,
                            SCMP_A0(SCMP_CMP_EQ, fd),
                            SCMP_A1(SCMP_CMP_EQ, (scmp_datum_t)buf),
                            SCMP_A2(SCMP_CMP_LE, BUF_SIZE));

To allow only empty string ("") as second fstatat argument, but as that empty string is passed as pointer I could only compare it to that pointer (which is random). More explanation in article Deep argument inspection for seccomp on lwm.net