NaCl: Add sanity check for number of open FDs at startup

This is primarily for Non-SFI NaCl, where leaking FDs would be a
security hole.  For SFI NaCl, this is just for defence in depth.

I've put the check just before enabling the seccomp-bpf sandbox.  This
guards against creation of unusual FDs, e.g. via epoll_create(), which
might happen even after enabling the SUID sandbox (which mostly disables
open()).

BUG=358719
TEST=browser_tests

Review URL: https://codereview.chromium.org/276443003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271583 0039d316-1c4b-4281-b951-d872f2087c98

components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc

@@ -46,7 +46,8 @@ NaClSandbox::NaClSandbox()
       layer_one_sealed_(false),
       layer_two_enabled_(false),
       layer_two_is_nonsfi_(false),
-      proc_fd_(-1) {
+      proc_fd_(-1),
+      setuid_sandbox_client_(sandbox::SetuidSandboxClient::Create()) {
   proc_fd_.reset(
       HANDLE_EINTR(open("/proc", O_DIRECTORY | O_RDONLY | O_CLOEXEC)));
   PCHECK(proc_fd_.is_valid());
@@ -72,29 +73,47 @@ bool NaClSandbox::HasOpenDirectory() {
 void NaClSandbox::InitializeLayerOneSandbox() {
   // Check that IsSandboxed() works. We should not be sandboxed at this point.
   CHECK(!IsSandboxed()) << "Unexpectedly sandboxed!";
-  scoped_ptr<sandbox::SetuidSandboxClient> setuid_sandbox_client(
-      sandbox::SetuidSandboxClient::Create());
-  const bool suid_sandbox_child = setuid_sandbox_client->IsSuidSandboxChild();
 
-  if (suid_sandbox_child) {
-    setuid_sandbox_client->CloseDummyFile();
+  if (setuid_sandbox_client_->IsSuidSandboxChild()) {
+    setuid_sandbox_client_->CloseDummyFile();
 
     // Make sure that no directory file descriptor is open, as it would bypass
     // the setuid sandbox model.
     CHECK(!HasOpenDirectory());
 
     // Get sandboxed.
-    CHECK(setuid_sandbox_client->ChrootMe());
+    CHECK(setuid_sandbox_client_->ChrootMe());
     CHECK(IsSandboxed());
     layer_one_enabled_ = true;
   }
 }
 
+void NaClSandbox::CheckForExpectedNumberOfOpenFds() {
+  if (setuid_sandbox_client_->IsSuidSandboxChild()) {
+    // We expect to have the following FDs open:
+    //  1-3) stdin, stdout, stderr.
+    //  4) The /dev/urandom FD used by base::GetUrandomFD().
+    //  5) A dummy pipe FD used to overwrite kSandboxIPCChannel.
+    //  6) The socket created by the SUID sandbox helper, used by ChrootMe().
+    //     After ChrootMe(), this is no longer connected to anything.
+    //     (Only present when running under the SUID sandbox.)
+    //  7) The socket for the Chrome IPC channel that's connected to the
+    //     browser process, kPrimaryIPCChannel.
+    //
+    // This sanity check ensures that dynamically loaded libraries don't
+    // leave any FDs open before we enable the sandbox.
+    sandbox::Credentials credentials;
+    CHECK_EQ(7, credentials.CountOpenFds(proc_fd_.get()));
+  }
+}
+
 void NaClSandbox::InitializeLayerTwoSandbox(bool uses_nonsfi_mode) {
   // seccomp-bpf only applies to the current thread, so it's critical to only
   // have a single thread running here.
   DCHECK(!layer_one_sealed_);
   CHECK(IsSingleThreaded());
+  CheckForExpectedNumberOfOpenFds();
+
   if (uses_nonsfi_mode) {
     layer_two_enabled_ = nacl::nonsfi::InitializeBPFSandbox();
     layer_two_is_nonsfi_ = true;

components/nacl/loader/sandbox_linux/nacl_sandbox_linux.h

@@ -7,6 +7,11 @@
 
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace sandbox {
+class SetuidSandboxClient;
+}
 
 namespace nacl {
 
@@ -61,6 +66,8 @@ class NaClSandbox {
   bool layer_two_enabled() { return layer_two_enabled_; }
 
  private:
+  void CheckForExpectedNumberOfOpenFds();
+
   bool layer_one_enabled_;
   bool layer_one_sealed_;
   bool layer_two_enabled_;
@@ -68,6 +75,7 @@ class NaClSandbox {
   // |proc_fd_| must be released before the layer-1 sandbox is considered
   // enforcing.
   base::ScopedFD proc_fd_;
+  scoped_ptr<sandbox::SetuidSandboxClient> setuid_sandbox_client_;
   DISALLOW_COPY_AND_ASSIGN(NaClSandbox);
 };
 

sandbox/linux/services/credentials.cc

@@ -173,6 +173,35 @@ Credentials::Credentials() {
 Credentials::~Credentials() {
 }
 
+int Credentials::CountOpenFds(int proc_fd) {
+  DCHECK_LE(0, proc_fd);
+  int proc_self_fd = openat(proc_fd, "self/fd", O_DIRECTORY | O_RDONLY);
+  PCHECK(0 <= proc_self_fd);
+
+  // Ownership of proc_self_fd is transferred here, it must not be closed
+  // or modified afterwards except via dir.
+  ScopedDIR dir(fdopendir(proc_self_fd));
+  CHECK(dir);
+
+  int count = 0;
+  struct dirent e;
+  struct dirent* de;
+  while (!readdir_r(dir.get(), &e, &de) && de) {
+    if (strcmp(e.d_name, ".") == 0 || strcmp(e.d_name, "..") == 0) {
+      continue;
+    }
+
+    int fd_num;
+    CHECK(base::StringToInt(e.d_name, &fd_num));
+    if (fd_num == proc_fd || fd_num == proc_self_fd) {
+      continue;
+    }
+
+    ++count;
+  }
+  return count;
+}
+
 bool Credentials::HasOpenDirectory(int proc_fd) {
   int proc_self_fd = -1;
   if (proc_fd >= 0) {

sandbox/linux/services/credentials.h

@@ -27,6 +27,11 @@ class SANDBOX_EXPORT Credentials {
   Credentials();
   ~Credentials();
 
+  // Returns the number of file descriptors in the current process's FD
+  // table, excluding |proc_fd|, which should be a file descriptor for
+  // /proc.
+  int CountOpenFds(int proc_fd);
+
   // Checks whether the current process has any directory file descriptor open.
   // Directory file descriptors are "capabilities" that would let a process use
   // system calls such as openat() to bypass restrictions such as

sandbox/linux/services/credentials_unittest.cc

@@ -57,6 +57,18 @@ TEST(Credentials, CreateAndDestroy) {
   scoped_ptr<Credentials> cred2(new Credentials);
 }
 
+TEST(Credentials, CountOpenFds) {
+  base::ScopedFD proc_fd(open("/proc", O_RDONLY | O_DIRECTORY));
+  ASSERT_TRUE(proc_fd.is_valid());
+  Credentials creds;
+  int fd_count = creds.CountOpenFds(proc_fd.get());
+  int fd = open("/dev/null", O_RDONLY);
+  ASSERT_LE(0, fd);
+  EXPECT_EQ(fd_count + 1, creds.CountOpenFds(proc_fd.get()));
+  ASSERT_EQ(0, IGNORE_EINTR(close(fd)));
+  EXPECT_EQ(fd_count, creds.CountOpenFds(proc_fd.get()));
+}
+
 TEST(Credentials, HasOpenDirectory) {
   Credentials creds;
   // No open directory should exist at startup.