Bug 1328896 - Restrict fcntl() in sandboxed content processes. r?gcp
MozReview-Commit-ID: BDBTwlT82mf
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -58,16 +58,20 @@ using namespace sandbox::bpf_dsl;
#ifndef MADV_FREE
#define MADV_FREE 8
#endif
#ifndef PR_SET_PTRACER
#define PR_SET_PTRACER 0x59616d61
#endif
+// The headers define O_LARGEFILE as 0 on x86_64, but we need the
+// actual value because it shows up in file flags.
+#define O_LARGEFILE_REAL 00100000
+
// To avoid visual confusion between "ifdef ANDROID" and "ifndef ANDROID":
#ifndef ANDROID
#define DESKTOP
#endif
// This file defines the seccomp-bpf system call filter policies.
// See also SandboxFilterUtil.h, for the CASES_FOR_* macros and
// SandboxFilterBase::Evaluate{Socket,Ipc}Call.
@@ -740,21 +744,45 @@ public:
.ElseIf(request == FIONREAD, Allow())
// Allow anything that isn't a tty ioctl, for now; bug 1302711
// will cover changing this to a default-deny policy.
.ElseIf(shifted_type != kTtyIoctls, Allow())
.Else(SandboxPolicyCommon::EvaluateSyscall(sysno));
}
#endif // !MOZ_ALSA
- CASES_FOR_fcntl:
- // Some fcntls have significant side effects like sending
- // arbitrary signals, and there's probably nontrivial kernel
- // attack surface; this should be locked down more if possible.
- return Allow();
+ CASES_FOR_fcntl: {
+ Arg<int> cmd(1);
+ Arg<int> flags(2);
+ // Typical use of F_SETFL is to modify the flags returned by
+ // F_GETFL and write them back, including some flags that
+ // F_SETFL ignores. This is a default-deny policy in case any
+ // new SETFL-able flags are added. (In particular we want to
+ // forbid O_ASYNC; see bug 1328896, but also see bug 1408438.)
+ static const int ignored_flags = O_ACCMODE | O_LARGEFILE_REAL | O_CLOEXEC;
+ static const int allowed_flags = ignored_flags | O_APPEND | O_NONBLOCK;
+ return Switch(cmd)
+ // Close-on-exec is meaningless when execve isn't allowed, but
+ // NSPR reads the bit and asserts that it has the expected value.
+ .Case(F_GETFD, Allow())
+ .Case(F_SETFD,
+ If((flags & ~FD_CLOEXEC) == 0, Allow())
+ .Else(InvalidSyscall()))
+ .Case(F_GETFL, Allow())
+ .Case(F_SETFL,
+ If((flags & ~allowed_flags) == 0, Allow())
+ .Else(InvalidSyscall()))
+ .Case(F_DUPFD_CLOEXEC, Allow())
+ // Pulseaudio uses F_SETLKW.
+ .Case(F_SETLKW, Allow())
+#ifdef F_SETLKW64
+ .Case(F_SETLKW64, Allow())
+#endif
+ .Default(SandboxPolicyCommon::EvaluateSyscall(sysno));
+ }
case __NR_mprotect:
case __NR_brk:
case __NR_madvise:
// libc's realloc uses mremap (Bug 1286119); wasm does too (bug 1342385).
case __NR_mremap:
return Allow();
--- a/security/sandbox/linux/reporter/SandboxReporter.cpp
+++ b/security/sandbox/linux/reporter/SandboxReporter.cpp
@@ -195,16 +195,17 @@ SubmitToTelemetry(const SandboxReport& a
} \
break
// The syscalls handled specially:
ARG_HEX(clone, 0); // flags
ARG_DECIMAL(prctl, 0); // option
ARG_HEX(ioctl, 1); // request
+ ARG_DECIMAL(fcntl, 1); // cmd
ARG_DECIMAL(madvise, 2); // advice
ARG_CLOCKID(clock_gettime, 0); // clk_id
#ifdef __NR_socketcall
ARG_DECIMAL(socketcall, 0); // call
#endif
#ifdef __NR_ipc
ARG_DECIMAL(ipc, 0); // call