Bug 1639181 - Allow a safe subset of fd flag fcntls in the common sandbox policy. r=gcp

Content processes allow a restricted subset of F_{GET,SET}{FD,FL} that
prevents setting unknown or known-unsafe flags, which was copied to the
socket process policy; this patch moves it to the common policy and
removes RDD's copy of GMP's override.

The immediate reason for this is DMD using F_GETFL via fdopen to use a
file descriptor passed over IPC, but in general this should be safe and
it's a reasonable thing to expect to be able to use.

Differential Revision: https://phabricator.services.mozilla.com/D77379
This commit is contained in:
Jed Davis 2020-05-29 18:18:43 +00:00
parent 42508ee26a
commit 9c23d852e1

View File

@ -532,6 +532,31 @@ class SandboxPolicyCommon : public SandboxPolicyBase {
CASES_FOR_fstat:
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()))
// F_GETFL is also used by fdopen
.Case(F_GETFL, Allow())
.Case(F_SETFL, If((flags & ~allowed_flags) == 0, Allow())
.Else(InvalidSyscall()))
.Default(SandboxPolicyBase::EvaluateSyscall(sysno));
}
// Simple I/O
case __NR_pread64:
case __NR_write:
@ -1105,25 +1130,7 @@ class ContentSandboxPolicy : public SandboxPolicyCommon {
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())
// Nvidia GL and fontconfig (newer versions) use fcntl file locking.
.Case(F_SETLK, Allow())
@ -1486,29 +1493,11 @@ class RDDSandboxPolicy final : public SandboxPolicyCommon {
: SandboxPolicyCommon(aBroker, ShmemUsage::MAY_CREATE,
AllowUnsafeSocketPair::NO) {}
static intptr_t FcntlTrap(const sandbox::arch_seccomp_data& aArgs,
void* aux) {
const auto cmd = static_cast<int>(aArgs.args[1]);
switch (cmd) {
// This process can't exec, so the actual close-on-exec flag
// doesn't matter; have it always read as true and ignore writes.
case F_GETFD:
return O_CLOEXEC;
case F_SETFD:
return 0;
default:
return -ENOSYS;
}
}
ResultExpr EvaluateSyscall(int sysno) const override {
switch (sysno) {
case __NR_getrusage:
return Allow();
CASES_FOR_fcntl:
return Trap(FcntlTrap, nullptr);
// Pass through the common policy.
default:
return SandboxPolicyCommon::EvaluateSyscall(sysno);
@ -1618,25 +1607,7 @@ class SocketProcessSandboxPolicy final : public SandboxPolicyCommon {
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())
// Nvidia GL and fontconfig (newer versions) use fcntl file locking.
.Case(F_SETLK, Allow())