Fix text rendering in QtWebEngine: https://issues.guix.gnu.org/52672 Patch copied from upstream: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/374232 From be0320febb694d850b76396346ef7ba5b45b7f0d Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 16 Sep 2021 13:17:58 +0200 Subject: [PATCH] [Backport] Linux sandbox: fix fstatat() crash This is a reland of https://crrev.com/c/2801873. Glibc has started rewriting fstat(fd, stat_buf) to fstatat(fd, "", stat_buf, AT_EMPTY_PATH). This works because when AT_EMPTY_PATH is specified, and the second argument is an empty string, then fstatat just performs an fstat on fd like normal. Unfortunately, fstatat() also allows stat-ing arbitrary pathnames like with fstatat(AT_FDCWD, "/i/am/a/file", stat_buf, 0); The baseline policy needs to prevent this usage of fstatat() since it doesn't allow access to arbitrary pathnames. Sadly, if the second argument is not an empty string, AT_EMPTY_PATH is simply ignored by current kernels. This means fstatat() is completely unsandboxable with seccomp, since we *need* to verify that the second argument is the empty string, but we can't dereference pointers in seccomp (due to limitations of BPF, and the difficulty of addressing these limitations due to TOCTOU issues). So, this CL Traps (raises a SIGSYS via seccomp) on any fstatat syscall. The signal handler, which runs in the sandboxed process, checks for AT_EMPTY_PATH and the empty string, and then rewrites any applicable fstatat() back into the old-style fstat(). Bug: 1164975 Change-Id: I3df6c04c0d781eb1f181d707ccaaead779337291 Reviewed-by: Robert Sesek Commit-Queue: Matthew Denton Cr-Commit-Position: refs/heads/master@{#903873} Reviewed-by: Peter Varga --- diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc index 3c67b12..ca19290 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc @@ -20,6 +20,7 @@ #include "sandbox/linux/seccomp-bpf-helpers/syscall_sets.h" #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h" #include "sandbox/linux/services/syscall_wrappers.h" +#include "sandbox/linux/system_headers/linux_stat.h" #include "sandbox/linux/system_headers/linux_syscalls.h" #if !defined(SO_PEEK_OFF) @@ -257,6 +258,13 @@ return RestrictKillTarget(current_pid, sysno); } + // The fstatat syscalls are file system syscalls, which will be denied below + // with fs_denied_errno. However some allowed fstat syscalls are rewritten by + // libc implementations to fstatat syscalls, and we need to rewrite them back. + if (sysno == __NR_fstatat_default) { + return RewriteFstatatSIGSYS(fs_denied_errno); + } + if (SyscallSets::IsFileSystem(sysno) || SyscallSets::IsCurrentDirectory(sysno)) { return Error(fs_denied_errno); diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc index 64ec1ce..814b700 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc @@ -50,7 +50,8 @@ namespace { -// This also tests that read(), write() and fstat() are allowed. +// This also tests that read(), write(), fstat(), and fstatat(.., "", .., +// AT_EMPTY_PATH) are allowed. void TestPipeOrSocketPair(base::ScopedFD read_end, base::ScopedFD write_end) { BPF_ASSERT_LE(0, read_end.get()); BPF_ASSERT_LE(0, write_end.get()); @@ -59,6 +60,20 @@ BPF_ASSERT_EQ(0, sys_ret); BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode)); + sys_ret = fstatat(read_end.get(), "", &stat_buf, AT_EMPTY_PATH); + BPF_ASSERT_EQ(0, sys_ret); + BPF_ASSERT(S_ISFIFO(stat_buf.st_mode) || S_ISSOCK(stat_buf.st_mode)); + + // Make sure fstatat with anything other than an empty string is denied. + sys_ret = fstatat(read_end.get(), "/", &stat_buf, AT_EMPTY_PATH); + BPF_ASSERT_EQ(sys_ret, -1); + BPF_ASSERT_EQ(EPERM, errno); + + // Make sure fstatat without AT_EMPTY_PATH is denied. + sys_ret = fstatat(read_end.get(), "", &stat_buf, 0); + BPF_ASSERT_EQ(sys_ret, -1); + BPF_ASSERT_EQ(EPERM, errno); + const ssize_t kTestTransferSize = 4; static const char kTestString[kTestTransferSize] = {'T', 'E', 'S', 'T'}; ssize_t transfered = 0; diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc index 76eb324..13e7180 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc @@ -6,6 +6,7 @@ #include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h" +#include #include #include #include @@ -22,6 +23,7 @@ #include "sandbox/linux/seccomp-bpf/syscall.h" #include "sandbox/linux/services/syscall_wrappers.h" #include "sandbox/linux/system_headers/linux_seccomp.h" +#include "sandbox/linux/system_headers/linux_stat.h" #include "sandbox/linux/system_headers/linux_syscalls.h" #if defined(__mips__) @@ -355,6 +357,24 @@ return -ENOSYS; } +intptr_t SIGSYSFstatatHandler(const struct arch_seccomp_data& args, + void* fs_denied_errno) { + if (args.nr == __NR_fstatat_default) { + if (*reinterpret_cast(args.args[1]) == '\0' && + args.args[3] == static_cast(AT_EMPTY_PATH)) { + return syscall(__NR_fstat_default, static_cast(args.args[0]), + reinterpret_cast(args.args[2])); + } + return -reinterpret_cast(fs_denied_errno); + } + + CrashSIGSYS_Handler(args, fs_denied_errno); + + // Should never be reached. + RAW_CHECK(false); + return -ENOSYS; +} + bpf_dsl::ResultExpr CrashSIGSYS() { return bpf_dsl::Trap(CrashSIGSYS_Handler, NULL); } @@ -387,6 +407,11 @@ return bpf_dsl::Trap(SIGSYSSchedHandler, NULL); } +bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno) { + return bpf_dsl::Trap(SIGSYSFstatatHandler, + reinterpret_cast(fs_denied_errno)); +} + void AllocateCrashKeys() { #if !defined(OS_NACL_NONSFI) if (seccomp_crash_key) diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h index 7a958b9..8cd735c 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h @@ -62,6 +62,19 @@ // sched_setparam(), sched_setscheduler() SANDBOX_EXPORT intptr_t SIGSYSSchedHandler(const arch_seccomp_data& args, void* aux); +// If the fstatat() syscall is functionally equivalent to an fstat() syscall, +// then rewrite the syscall to the equivalent fstat() syscall which can be +// adequately sandboxed. +// If the fstatat() is not functionally equivalent to an fstat() syscall, we +// fail with -fs_denied_errno. +// If the syscall is not an fstatat() at all, crash in the same way as +// CrashSIGSYS_Handler. +// This is necessary because glibc and musl have started rewriting fstat(fd, +// stat_buf) as fstatat(fd, "", stat_buf, AT_EMPTY_PATH). We rewrite the latter +// back to the former, which is actually sandboxable. +SANDBOX_EXPORT intptr_t +SIGSYSFstatatHandler(const struct arch_seccomp_data& args, + void* fs_denied_errno); // Variants of the above functions for use with bpf_dsl. SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYS(); @@ -72,6 +85,7 @@ SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSFutex(); SANDBOX_EXPORT bpf_dsl::ResultExpr CrashSIGSYSPtrace(); SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteSchedSIGSYS(); +SANDBOX_EXPORT bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno); // Allocates a crash key so that Seccomp information can be recorded. void AllocateCrashKeys(); diff --git a/chromium/sandbox/linux/syscall_broker/broker_process.cc b/chromium/sandbox/linux/syscall_broker/broker_process.cc index d72c9d2..36df5e4 100644 --- a/chromium/sandbox/linux/syscall_broker/broker_process.cc +++ b/chromium/sandbox/linux/syscall_broker/broker_process.cc @@ -122,44 +122,49 @@ } bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const { + // The syscalls unavailable on aarch64 are all blocked by Android's default + // seccomp policy, even on non-aarch64 architectures. I.e., the syscalls XX() + // with a corresponding XXat() versions are typically unavailable in aarch64 + // and are default disabled in Android. So, we should refuse to broker them + // to be consistent with the platform's restrictions. switch (sysno) { -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_access: #endif case __NR_faccessat: return !fast_check || allowed_command_set_.test(COMMAND_ACCESS); -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_mkdir: #endif case __NR_mkdirat: return !fast_check || allowed_command_set_.test(COMMAND_MKDIR); -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_open: #endif case __NR_openat: return !fast_check || allowed_command_set_.test(COMMAND_OPEN); -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_readlink: #endif case __NR_readlinkat: return !fast_check || allowed_command_set_.test(COMMAND_READLINK); -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_rename: #endif case __NR_renameat: case __NR_renameat2: return !fast_check || allowed_command_set_.test(COMMAND_RENAME); -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_rmdir: return !fast_check || allowed_command_set_.test(COMMAND_RMDIR); #endif -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_stat: case __NR_lstat: #endif @@ -184,7 +189,7 @@ return !fast_check || allowed_command_set_.test(COMMAND_STAT); #endif -#if !defined(__aarch64__) +#if !defined(__aarch64__) && !defined(OS_ANDROID) case __NR_unlink: return !fast_check || allowed_command_set_.test(COMMAND_UNLINK); #endif diff --git a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc index b1d7106..15e00d5 100644 --- a/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc +++ b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc @@ -1596,52 +1596,52 @@ const base::flat_map> kSysnosForCommand = { {COMMAND_ACCESS, {__NR_faccessat, -#if defined(__NR_access) +#if defined(__NR_access) && !defined(OS_ANDROID) __NR_access #endif }}, {COMMAND_MKDIR, {__NR_mkdirat, -#if defined(__NR_mkdir) +#if defined(__NR_mkdir) && !defined(OS_ANDROID) __NR_mkdir #endif }}, {COMMAND_OPEN, {__NR_openat, -#if defined(__NR_open) +#if defined(__NR_open) && !defined(OS_ANDROID) __NR_open #endif }}, {COMMAND_READLINK, {__NR_readlinkat, -#if defined(__NR_readlink) +#if defined(__NR_readlink) && !defined(OS_ANDROID) __NR_readlink #endif }}, {COMMAND_RENAME, {__NR_renameat, -#if defined(__NR_rename) +#if defined(__NR_rename) && !defined(OS_ANDROID) __NR_rename #endif }}, {COMMAND_UNLINK, {__NR_unlinkat, -#if defined(__NR_unlink) +#if defined(__NR_unlink) && !defined(OS_ANDROID) __NR_unlink #endif }}, {COMMAND_RMDIR, {__NR_unlinkat, -#if defined(__NR_rmdir) +#if defined(__NR_rmdir) && !defined(OS_ANDROID) __NR_rmdir #endif }}, {COMMAND_STAT, { -#if defined(__NR_stat) +#if defined(__NR_stat) && !defined(OS_ANDROID) __NR_stat, #endif -#if defined(__NR_lstat) +#if defined(__NR_lstat) && !defined(OS_ANDROID) __NR_lstat, #endif #if defined(__NR_fstatat) diff --git a/chromium/sandbox/linux/system_headers/linux_stat.h b/chromium/sandbox/linux/system_headers/linux_stat.h index 35788eb..83b89ef 100644 --- a/chromium/sandbox/linux/system_headers/linux_stat.h +++ b/chromium/sandbox/linux/system_headers/linux_stat.h @@ -157,6 +157,10 @@ }; #endif +#if !defined(AT_EMPTY_PATH) +#define AT_EMPTY_PATH 0x1000 +#endif + // On 32-bit systems, we default to the 64-bit stat struct like libc // implementations do. Otherwise we default to the normal stat struct which is // already 64-bit.