Remove a possible race in the SUID sandbox (minor)

The SUID sandbox can be used to set the oom_adj value for non-dumpable
processes owned by the same user. When doing so, we previously first
checked the directory owner and then opened the oom_adj file. In between
the check and the open, the process could have died and another process
could have taken that PID value. We would then adjust the OOM value of
the wrong process.

Given how PIDs are allocated, this is very hard to exploit and, even
then, a minor security issue at best, but we can avoid the issue
entirely with openat.

http://codereview.chromium.org/2118007

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

1 files changed, 18 insertions, 7 deletions

diff --git a/sandbox/linux/suid/process_util_linux.c b/sandbox/linux/suid/process_util_linux.c
index 7b31274..17453de 100644
--- a/sandbox/linux/suid/process_util_linux.c
+++ b/sandbox/linux/suid/process_util_linux.c
@@ -5,6 +5,8 @@
 // The following is the C version of code from base/process_utils_linux.cc.
 // We shouldn't link against C++ code in a setuid binary.
 
+#define _GNU_SOURCE  // needed for O_DIRECTORY
+
 #include "process_util.h"
 
 #include <fcntl.h>
@@ -21,24 +23,33 @@ bool AdjustOOMScore(pid_t process, int score) {
   if (score < 0 || score > 15)
     return false;
 
-  char oom_adj[35];  // "/proc/" + log_10(2**64) + "/oom_adj\0"
-                     //    6     +       20     +     9         = 35
+  char oom_adj[27];  // "/proc/" + log_10(2**64) + "\0"
+                     //    6     +       20     +     1         = 27
   snprintf(oom_adj, sizeof(oom_adj), "/proc/%" PRIdMAX, (intmax_t)process);
 
+  const int dirfd = open(oom_adj, O_RDONLY | O_DIRECTORY);
+  if (dirfd < 0)
+    return false;
+
   struct stat statbuf;
-  if (stat(oom_adj, &statbuf) < 0)
+  if (fstat(dirfd, &statbuf) < 0) {
+    close(dirfd);
     return false;
-  if (getuid() != statbuf.st_uid)
+  }
+  if (getuid() != statbuf.st_uid) {
+    close(dirfd);
     return false;
+  }
+
+  const int fd = openat(dirfd, "oom_adj", O_WRONLY);
+  close(dirfd);
 
-  strcat(oom_adj, "/oom_adj");
-  int fd = open(oom_adj, O_WRONLY);
   if (fd < 0)
     return false;
 
   char buf[3];  // 0 <= |score| <= 15;
   snprintf(buf, sizeof(buf), "%d", score);
-  ssize_t len = strlen(buf);
+  size_t len = strlen(buf);
 
   ssize_t bytes_written = write(fd, buf, len);
   close(fd);