Bug 1328896 - Restrict fcntl() in sandboxed content processes. r=gcp
authorJed Davis <jld@mozilla.com>
Mon, 24 Jul 2017 17:33:07 -0600
changeset 387301 ff9088972319ed49297b6cbd134e1d3fb121ed64
parent 387300 36a03481388877058ced00d03297b918f0eda378
child 387302 37360be5d881acc3ebbf79de838c96b99edd3650
push id32716
push userarchaeopteryx@coole-files.de
push dateFri, 20 Oct 2017 09:38:32 +0000
treeherdermozilla-central@d1e995c8640a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1328896
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1328896 - Restrict fcntl() in sandboxed content processes. r=gcp MozReview-Commit-ID: BDBTwlT82mf
security/sandbox/linux/SandboxFilter.cpp
security/sandbox/linux/reporter/SandboxReporter.cpp
--- 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