Bug 1405891 - Block tty-related ioctl()s in sandboxed content processes. r=gcp
authorJed Davis <jld@mozilla.com>
Thu, 05 Oct 2017 19:53:31 -0600
changeset 435687 fb637352a959581a2c7e3fc02b4801e27518761b
parent 435686 e1a3a89200b725a0ddd52f5f7956b2d9a9f8c523
child 435688 9fc84ba52f31cb8640dd3077585fdd8fc53f7385
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1405891
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 1405891 - Block tty-related ioctl()s in sandboxed content processes. r=gcp MozReview-Commit-ID: KiBfibjLSfK
security/sandbox/linux/SandboxFilter.cpp
security/sandbox/linux/reporter/SandboxReporter.cpp
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -14,21 +14,23 @@
 #ifdef MOZ_GMP_SANDBOX
 #include "SandboxOpenedFiles.h"
 #endif
 #include "mozilla/PodOperations.h"
 #include "mozilla/UniquePtr.h"
 
 #include <errno.h>
 #include <fcntl.h>
+#include <linux/ioctl.h>
 #include <linux/ipc.h>
 #include <linux/net.h>
 #include <linux/prctl.h>
 #include <linux/sched.h>
 #include <string.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <sys/syscall.h>
 #include <sys/utsname.h>
 #include <time.h>
 #include <unistd.h>
 #include <vector>
 #include <algorithm>
@@ -314,17 +316,17 @@ public:
     case __NR_exit:
     case __NR_exit_group:
       return Allow();
 
 #ifdef MOZ_ASAN
       // ASAN's error reporter wants to know if stderr is a tty.
     case __NR_ioctl: {
       Arg<int> fd(0);
-      return If(fd == STDERR_FILENO, Allow())
+      return If(fd == STDERR_FILENO, Error(ENOTTY))
         .Else(InvalidSyscall());
     }
 
       // ...and before compiler-rt r209773, it will call readlink on
       // /proc/self/exe and use the cached value only if that fails:
     case __NR_readlink:
     case __NR_readlinkat:
       return Error(ENOENT);
@@ -707,21 +709,41 @@ public:
     case __NR_writev:
     case __NR_pread64:
 #ifdef DESKTOP
     case __NR_pwrite64:
     case __NR_readahead:
 #endif
       return Allow();
 
-    case __NR_ioctl:
-      // ioctl() is for GL. Remove when GL proxy is implemented.
-      // Additionally ioctl() might be a place where we want to have
-      // argument filtering
-      return Allow();
+    case __NR_ioctl: {
+      static const unsigned long kTypeMask = _IOC_TYPEMASK << _IOC_TYPESHIFT;
+      static const unsigned long kTtyIoctls = TIOCSTI & kTypeMask;
+      // On some older architectures (but not x86 or ARM), ioctls are
+      // assigned type fields differently, and the TIOC/TC/FIO group
+      // isn't all the same type.  If/when we support those archs,
+      // this would need to be revised (but really this should be a
+      // default-deny policy; see below).
+      static_assert(kTtyIoctls == (TCSETA & kTypeMask) &&
+                    kTtyIoctls == (FIOASYNC & kTypeMask),
+                    "tty-related ioctls use the same type");
+
+      Arg<unsigned long> request(1);
+      auto shifted_type = request & kTypeMask;
+
+      // Rust's stdlib seems to use FIOCLEX instead of equivalent fcntls.
+      return If(request == FIOCLEX, Allow())
+        // ffmpeg, and anything else that calls isatty(), will be told
+        // that nothing is a typewriter:
+        .ElseIf(request == TCGETS, Error(ENOTTY))
+        // 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));
+    }
 
     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();
 
     case __NR_mprotect:
--- a/security/sandbox/linux/reporter/SandboxReporter.cpp
+++ b/security/sandbox/linux/reporter/SandboxReporter.cpp
@@ -194,16 +194,17 @@ SubmitToTelemetry(const SandboxReport& a
         key.AppendInt(aReport.mArgs[idx]);                  \
       }                                                     \
       break
 
     // The syscalls handled specially:
 
     ARG_HEX(clone, 0); // flags
     ARG_DECIMAL(prctl, 0); // option
+    ARG_HEX(ioctl, 1); // request
     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