Bug 1328896 - Restrict fcntl() in sandboxed content processes. r?gcp draft
authorJed Davis <jld@mozilla.com>
Mon, 24 Jul 2017 17:33:07 -0600
changeset 682075 34483d5bb100aa79abaf2f8b4b4f3a90a6511560
parent 681681 7b75416fb54c6733b7403e340457007658c42c14
child 736297 9fc26b18ecc7b827510f2a38675c2ad0aedc06f4
push id84998
push userbmo:jld@mozilla.com
push dateWed, 18 Oct 2017 03:32:21 +0000
reviewersgcp
bugs1328896
milestone58.0a1
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