summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2021-09-16 13:17:58 +0200
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2021-10-04 10:21:04 +0200
commit0080853ab63f0f28b835360839df87868a76f396 (patch)
treeab664f448890db59b0df0a03d69880143746df78
parentb6fc11be60033b64b1dc20eeafd706cf742c9e81 (diff)
downloadqtwebengine-chromium-0080853ab63f0f28b835360839df87868a76f396.tar.gz
[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-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}
-rw-r--r--chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc8
-rw-r--r--chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc17
-rw-r--r--chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc25
-rw-r--r--chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h14
-rw-r--r--chromium/sandbox/linux/syscall_broker/broker_process.cc21
-rw-r--r--chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc18
-rw-r--r--chromium/sandbox/linux/system_headers/linux_stat.h4
7 files changed, 89 insertions, 18 deletions
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
index 05c39f0f564..083c87990bf 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)
@@ -286,6 +287,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/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
index 68c29b564bb..57d307e09d3 100644
--- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc
+++ b/chromium/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/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc
index 76eb32493f5..13e71801f62 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 <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/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h
index 7a958b93b27..8cd735ce157 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 @@ 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/chromium/sandbox/linux/syscall_broker/broker_process.cc b/chromium/sandbox/linux/syscall_broker/broker_process.cc
index c2176eb785e..e9dad37485a 100644
--- a/chromium/sandbox/linux/syscall_broker/broker_process.cc
+++ b/chromium/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/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc b/chromium/sandbox/linux/syscall_broker/broker_process_unittest.cc
index b1d71063409..15e00d56815 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 @@ 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/chromium/sandbox/linux/system_headers/linux_stat.h b/chromium/sandbox/linux/system_headers/linux_stat.h
index 35788eb22a4..83b89efc75e 100644
--- a/chromium/sandbox/linux/system_headers/linux_stat.h
+++ b/chromium/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.