setuid tools: open /proc directories relatively

Fix a race where we could end-up opening the wrong /proc/pid/fd because
we were using absolute paths.

BUG=162489
NOTRY=true


Review URL: https://chromiumcodereview.appspot.com/11418160

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

1 files changed, 44 insertions, 21 deletions

diff --git a/sandbox/linux/suid/linux_util.c b/sandbox/linux/suid/linux_util.c
index c5af0d0..256468f 100644
--- a/sandbox/linux/suid/linux_util.c
+++ b/sandbox/linux/suid/linux_util.c
@@ -5,9 +5,12 @@
 // The following is duplicated from base/linux_utils.cc.
 // We shouldn't link against C++ code in a setuid binary.
 
+#define _GNU_SOURCE  // For O_DIRECTORY
 #include "linux_util.h"
 
 #include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -23,29 +26,42 @@ static const char kSocketLinkPrefix[] = "socket:[";
 // socket.
 //   inode_out: (output) set to the inode number on success
 //   path: e.g. /proc/1234/fd/5 (must be a UNIX domain socket descriptor)
-static bool ProcPathGetInode(ino_t* inode_out, const char* path) {
+static bool ProcPathGetInodeAt(ino_t* inode_out, int base_dir_fd,
+                               const char* path) {
+  // We also check that the path is relative.
+  if (!inode_out || !path || *path == '/')
+    return false;
   char buf[256];
-  const ssize_t n = readlink(path, buf, sizeof(buf) - 1);
-  if (n == -1)
+  const ssize_t n = readlinkat(base_dir_fd, path, buf, sizeof(buf) - 1);
+  if (n < 0)
     return false;
   buf[n] = 0;
 
   if (memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1))
     return false;
 
-  char *endptr;
-  const unsigned long long int inode_ul =
+  char *endptr = NULL;
+  errno = 0;
+  const unsigned long long int inode_ull =
       strtoull(buf + sizeof(kSocketLinkPrefix) - 1, &endptr, 10);
-  if (*endptr != ']')
-    return false;
-
-  if (inode_ul == ULLONG_MAX)
+  if (inode_ull == ULLONG_MAX || !endptr || *endptr != ']' || errno != 0)
     return false;
 
-  *inode_out = inode_ul;
+  *inode_out = inode_ull;
   return true;
 }
 
+static DIR* opendirat(int base_dir_fd, const char* name) {
+  // Also check that |name| is relative.
+  if (base_dir_fd < 0 || !name || *name == '/')
+    return NULL;
+  int new_dir_fd = openat(base_dir_fd, name, O_RDONLY | O_DIRECTORY);
+  if (new_dir_fd < 0)
+    return NULL;
+
+  return fdopendir(new_dir_fd);
+}
+
 bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode) {
   bool already_found = false;
 
@@ -56,9 +72,10 @@ bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode) {
   const uid_t uid = getuid();
   struct dirent* dent;
   while ((dent = readdir(proc))) {
-    char *endptr;
+    char *endptr = NULL;
+    errno = 0;
     const unsigned long int pid_ul = strtoul(dent->d_name, &endptr, 10);
-    if (pid_ul == ULONG_MAX || *endptr)
+    if (pid_ul == ULONG_MAX || !endptr || *endptr || errno != 0)
       continue;
 
     // We have this setuid code here because the zygote and its children have
@@ -66,34 +83,39 @@ bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode) {
     // extra check so users cannot accidentally gain information about other
     // users' processes. To determine process ownership, we use the property
     // that if user foo owns process N, then /proc/N is owned by foo.
+    int proc_pid_fd = -1;
     {
       char buf[256];
       struct stat statbuf;
       snprintf(buf, sizeof(buf), "/proc/%lu", pid_ul);
-      if (stat(buf, &statbuf) < 0)
+      proc_pid_fd = open(buf, O_RDONLY | O_DIRECTORY);
+      if (proc_pid_fd < 0)
         continue;
-      if (uid != statbuf.st_uid)
+      if (fstat(proc_pid_fd, &statbuf) < 0 || uid != statbuf.st_uid) {
+        close(proc_pid_fd);
         continue;
+      }
     }
 
-    char buf[256];
-    snprintf(buf, sizeof(buf), "/proc/%lu/fd", pid_ul);
-    DIR* fd = opendir(buf);
-    if (!fd)
+    DIR* fd = opendirat(proc_pid_fd, "fd");
+    if (!fd) {
+      close(proc_pid_fd);
       continue;
+    }
 
     while ((dent = readdir(fd))) {
-      int printed = snprintf(buf, sizeof(buf), "/proc/%lu/fd/%s", pid_ul,
-                             dent->d_name);
+      char buf[256];
+      int printed = snprintf(buf, sizeof(buf), "fd/%s", dent->d_name);
       if (printed < 0 || printed >= (int)(sizeof(buf) - 1)) {
         continue;
       }
 
       ino_t fd_inode;
-      if (ProcPathGetInode(&fd_inode, buf)) {
+      if (ProcPathGetInodeAt(&fd_inode, proc_pid_fd, buf)) {
         if (fd_inode == socket_inode) {
           if (already_found) {
             closedir(fd);
+            close(proc_pid_fd);
             closedir(proc);
             return false;
           }
@@ -105,6 +127,7 @@ bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode) {
       }
     }
     closedir(fd);
+    close(proc_pid_fd);
   }
   closedir(proc);