86 lines
3.1 KiB
Diff
86 lines
3.1 KiB
Diff
|
|
# HG changeset patch
|
|
# User Jed Davis <jld@mozilla.com>
|
|
# Date 1603993288 0
|
|
# Node ID 3962fa9f3084c5bca8458cecdaac664dc431164a
|
|
# Parent 4e708898d49e078de5f946d03b0627779b282fd0
|
|
Bug 1673770 - Extend the handling of fstatat-as-fstat to sandboxes that don't use a file broker. r=gcp, a=RyanVM
|
|
|
|
The fix for bug 1660901, to handle the subset of fstatat that is
|
|
equivalent to fstat, was incomplete: it was added to the existing
|
|
hook for the file broker, so processes that don't use a broker (like
|
|
GMP) didn't get the fix. That wasn't a problem when the only use of
|
|
that feature was in content processes via GTK, but now that glibc has
|
|
reimplemented fstat that way, it's necessary for all processes.
|
|
|
|
Differential Revision: https://phabricator.services.mozilla.com/D95108
|
|
|
|
diff --git a/security/sandbox/linux/SandboxFilter.cpp b/security/sandbox/linux/SandboxFilter.cpp
|
|
--- a/security/sandbox/linux/SandboxFilter.cpp
|
|
+++ b/security/sandbox/linux/SandboxFilter.cpp
|
|
@@ -248,16 +248,20 @@ class SandboxPolicyCommon : public Sandb
|
|
strcmp(path, "") == 0) {
|
|
#ifdef __NR_fstat64
|
|
return DoSyscall(__NR_fstat64, fd, buf);
|
|
#else
|
|
return DoSyscall(__NR_fstat, fd, buf);
|
|
#endif
|
|
}
|
|
|
|
+ if (!broker) {
|
|
+ return BlockedSyscallTrap(aArgs, nullptr);
|
|
+ }
|
|
+
|
|
if (fd != AT_FDCWD && path[0] != '/') {
|
|
SANDBOX_LOG_ERROR("unsupported fd-relative fstatat(%d, \"%s\", %p, %d)",
|
|
fd, path, buf, flags);
|
|
return BlockedSyscallTrap(aArgs, nullptr);
|
|
}
|
|
if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)) != 0) {
|
|
SANDBOX_LOG_ERROR("unsupported flags %d in fstatat(%d, \"%s\", %p, %d)",
|
|
(flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT)), fd,
|
|
@@ -442,17 +446,17 @@ class SandboxPolicyCommon : public Sandb
|
|
return Nothing();
|
|
}
|
|
}
|
|
|
|
ResultExpr EvaluateSyscall(int sysno) const override {
|
|
// If a file broker client was provided, route syscalls to it;
|
|
// otherwise, fall through to the main policy, which will deny
|
|
// them.
|
|
- if (mBroker != nullptr) {
|
|
+ if (mBroker) {
|
|
switch (sysno) {
|
|
case __NR_open:
|
|
return Trap(OpenTrap, mBroker);
|
|
case __NR_openat:
|
|
return Trap(OpenAtTrap, mBroker);
|
|
case __NR_access:
|
|
return Trap(AccessTrap, mBroker);
|
|
case __NR_faccessat:
|
|
@@ -481,16 +485,23 @@ class SandboxPolicyCommon : public Sandb
|
|
return Trap(RmdirTrap, mBroker);
|
|
case __NR_unlink:
|
|
return Trap(UnlinkTrap, mBroker);
|
|
case __NR_readlink:
|
|
return Trap(ReadlinkTrap, mBroker);
|
|
case __NR_readlinkat:
|
|
return Trap(ReadlinkAtTrap, mBroker);
|
|
}
|
|
+ } else {
|
|
+ // In the absence of a broker we still need to handle the
|
|
+ // fstat-equivalent subset of fstatat; see bug 1673770.
|
|
+ switch (sysno) {
|
|
+ CASES_FOR_fstatat:
|
|
+ return Trap(StatAtTrap, nullptr);
|
|
+ }
|
|
}
|
|
|
|
switch (sysno) {
|
|
// Timekeeping
|
|
case __NR_clock_nanosleep:
|
|
case __NR_clock_getres:
|
|
#ifdef __NR_clock_gettime64
|
|
case __NR_clock_gettime64:
|
|
|