Bug 1405891 - Block tty-related ioctl()s in sandboxed content processes. r?gcp
MozReview-Commit-ID: KiBfibjLSfK
--- 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