diff options
Diffstat (limited to 'chromium-ungoogled/build/patches/linux-sandbox-fix-fstatat-crash.patch')
| -rw-r--r-- | chromium-ungoogled/build/patches/linux-sandbox-fix-fstatat-crash.patch | 348 |
1 files changed, 348 insertions, 0 deletions
diff --git a/chromium-ungoogled/build/patches/linux-sandbox-fix-fstatat-crash.patch b/chromium-ungoogled/build/patches/linux-sandbox-fix-fstatat-crash.patch new file mode 100644 index 00000000..899e48e6 --- /dev/null +++ b/chromium-ungoogled/build/patches/linux-sandbox-fix-fstatat-crash.patch @@ -0,0 +1,348 @@ +From 60d5e803ef2a4874d29799b638754152285e0ed9 Mon Sep 17 00:00:00 2001 +From: Matthew Denton <mpdenton@chromium.org> +Date: Wed, 21 Jul 2021 12:55:11 +0000 +Subject: [PATCH] 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-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042179 +Reviewed-by: Robert Sesek <rsesek@chromium.org> +Commit-Queue: Matthew Denton <mpdenton@chromium.org> +Cr-Commit-Position: refs/heads/master@{#903873} +--- + .../seccomp-bpf-helpers/baseline_policy.cc | 8 ++++++ + .../baseline_policy_unittest.cc | 17 ++++++++++++- + .../seccomp-bpf-helpers/sigsys_handlers.cc | 25 +++++++++++++++++++ + .../seccomp-bpf-helpers/sigsys_handlers.h | 14 +++++++++++ + .../linux/syscall_broker/broker_process.cc | 21 ++++++++++------ + .../syscall_broker/broker_process_unittest.cc | 18 ++++++------- + sandbox/linux/system_headers/linux_stat.h | 4 +++ + 7 files changed, 89 insertions(+), 18 deletions(-) + +diff --git a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc +index f2a60bb4d7..9df0d2dbd3 100644 +--- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc ++++ b/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) +@@ -304,6 +305,13 @@ ResultExpr EvaluateSyscallImpl(int fs_denied_errno, + return Allow(); + } + ++ // 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/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc +index 68c29b564b..57d307e09d 100644 +--- a/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc ++++ b/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc +@@ -51,7 +51,8 @@ namespace sandbox { + + 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()); +@@ -60,6 +61,20 @@ void TestPipeOrSocketPair(base::ScopedFD read_end, base::ScopedFD write_end) { + 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/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc +index 64edbd68bd..71068a0452 100644 +--- a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc ++++ b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc +@@ -6,6 +6,7 @@ + + #include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h" + ++#include <fcntl.h> + #include <stddef.h> + #include <stdint.h> + #include <string.h> +@@ -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 @@ intptr_t SIGSYSSchedHandler(const struct arch_seccomp_data& args, + return -ENOSYS; + } + ++intptr_t SIGSYSFstatatHandler(const struct arch_seccomp_data& args, ++ void* fs_denied_errno) { ++ if (args.nr == __NR_fstatat_default) { ++ if (*reinterpret_cast<const char*>(args.args[1]) == '\0' && ++ args.args[3] == static_cast<uint64_t>(AT_EMPTY_PATH)) { ++ return syscall(__NR_fstat_default, static_cast<int>(args.args[0]), ++ reinterpret_cast<default_stat_struct*>(args.args[2])); ++ } ++ return -reinterpret_cast<intptr_t>(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 @@ bpf_dsl::ResultExpr RewriteSchedSIGSYS() { + return bpf_dsl::Trap(SIGSYSSchedHandler, NULL); + } + ++bpf_dsl::ResultExpr RewriteFstatatSIGSYS(int fs_denied_errno) { ++ return bpf_dsl::Trap(SIGSYSFstatatHandler, ++ reinterpret_cast<void*>(fs_denied_errno)); ++} ++ + void AllocateCrashKeys() { + #if !defined(OS_NACL_NONSFI) + if (seccomp_crash_key) +diff --git a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h +index 7a958b93b2..8cd735ce15 100644 +--- a/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h ++++ b/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h +@@ -62,6 +62,19 @@ SANDBOX_EXPORT intptr_t SIGSYSPtraceFailure(const arch_seccomp_data& args, + // 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 CrashSIGSYSKill(); + 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/sandbox/linux/syscall_broker/broker_process.cc b/sandbox/linux/syscall_broker/broker_process.cc +index c2176eb785..e9dad37485 100644 +--- a/sandbox/linux/syscall_broker/broker_process.cc ++++ b/sandbox/linux/syscall_broker/broker_process.cc +@@ -113,44 +113,49 @@ bool BrokerProcess::IsSyscallAllowed(int sysno) const { + } + + 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 +@@ -175,7 +180,7 @@ bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const { + 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/sandbox/linux/syscall_broker/broker_process_unittest.cc b/sandbox/linux/syscall_broker/broker_process_unittest.cc +index c65f25a78a..f0db08d84e 100644 +--- a/sandbox/linux/syscall_broker/broker_process_unittest.cc ++++ b/sandbox/linux/syscall_broker/broker_process_unittest.cc +@@ -1596,52 +1596,52 @@ TEST(BrokerProcess, IsSyscallAllowed) { + const base::flat_map<BrokerCommand, base::flat_set<int>> 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/sandbox/linux/system_headers/linux_stat.h b/sandbox/linux/system_headers/linux_stat.h +index 35788eb22a..83b89efc75 100644 +--- a/sandbox/linux/system_headers/linux_stat.h ++++ b/sandbox/linux/system_headers/linux_stat.h +@@ -157,6 +157,10 @@ struct kernel_stat { + }; + #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. |