Plumb sandbox rules through the helper executable.

chrome/app/chrome_exe_main_mac.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // The entry point for all Mac Chromium processes, including the outer app
 // bundle (browser) and helper app (renderer, plugin, and friends).
 
 #include <dlfcn.h>
 #include <errno.h>
 #include <libgen.h>
 #include <mach-o/dyld.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
+#include <string>
+#include <vector>
+
 #include "chrome/common/chrome_version.h"
 
+#if defined(HELPER_EXECUTABLE)
+#include "sandbox/mac/seatbelt_exec.h"
+#endif  // defined(HELPER_EXECUTABLE)
+
+extern char **environ;
+
+namespace {
+
 typedef int (*ChromeMainPtr)(int, char**);
 
+#if defined(HELPER_EXECUTABLE)
+// The name of the parameter containing the executable path.
+constexpr char exec_param[] = "EXECUTABLE_PATH";
+// The name of the parameter containing the PID of Chrome.
+constexpr char pid_param[] = "CHROMIUM_PID";
+// The command line parameter to engage the v2 sandbox.
+constexpr char v2_sandbox_arg[] = "--v2-sandbox";
+// The command line parameter for the file descriptor used to receive the
+// sandbox policy.
+constexpr char fd_mapping_arg[] = "--fd_mapping=";
+
+int SandboxExec(int argc, char* argv[], int fd_mapping) {
+  char rp[MAXPATHLEN];
+  if (realpath(argv[0], rp) == NULL)
+    abort();
+
+  sandbox::SeatbeltExecServer server(fd_mapping);
+
+  if (!server.SetParameter(exec_param, rp) ||
+      !server.SetParameter(pid_param, std::to_string(getpid()))) {
+    fprintf(stderr, "Failed to set up parameters for sandbox.\n");
+    return -1;

[Mark Mentovai, 2017/05/23 20:41:28]

This isn’t right for something that’s going to be used as main()’s exit code.

[Mark Mentovai, 2017/05/23 20:41:29]

I also don’t see any rhyme or reason to when you decided to return (and thus
exit) and when you decided to abort(). Maybe there’s a distinction between the
two that I don’t see.

[Greg K, 2017/05/23 23:31:29]

Yeah that was an oversight. Everything should just abort() if an unexpected
condition happens.

+  }
+
+  if (server.InitializeSandbox() != 0)
+    abort();
+
+  std::vector<char*> new_argv;
+  for (int i = 1; i < argc; ++i) {
+    if (strcmp(argv[i], v2_sandbox_arg) != 0 &&
+        strncmp(argv[i], fd_mapping_arg, strlen(fd_mapping_arg)) != 0) {
+      new_argv.push_back(argv[i]);
+    }
+  }
+  new_argv.push_back(nullptr);
+
+  // The helper executable re-executes itself under the sandbox.
+  execve(argv[0], new_argv.data(), environ);

[Mark Mentovai, 2017/05/23 20:41:29]

Instead of declaring environ and calling execve(), you can just call execv() and
let it deal with that for you.

[Mark Mentovai, 2017/05/23 20:41:29]

NSGetExecutablePath() would be better than argv[0], since argv[0] can really be
anything, including something that would make sense only given a $PATH search
(execvp()), but also including something entirely unrelated to the executable.

[Greg K, 2017/05/23 23:31:29]

Done.

[Greg K, 2017/05/23 23:31:29]

Done.

+  perror("execve");
+  return 1;
+}
+#endif  // defined(HELPER_EXECUTABLE)
+
+}  // namespace
+
 __attribute__((visibility("default"))) int main(int argc, char* argv[]) {
 #if defined(HELPER_EXECUTABLE)
+  bool enable_v2_sandbox = false;
+  int fd_mapping = -1;
+  for (int i = 1; i < argc; i++) {
+    if (strcmp(argv[i], v2_sandbox_arg) == 0) {
+      enable_v2_sandbox = true;
+    } else if (strncmp(argv[i], fd_mapping_arg, strlen(fd_mapping_arg)) == 0) {
+      // Parse --fd_mapping=X to get the file descriptor X.
+      std::string arg(argv[i]);
+      std::string fd_str = arg.substr(arg.find("=") + 1, arg.length());

[Mark Mentovai, 2017/05/23 20:41:28]

You don’t need to find the = now. You know where it is based on the length of
fd_mapping_arg.

[Greg K, 2017/05/23 23:31:29]

Done.

+      fd_mapping = std::stoi(fd_str);
+    }
+  }
+  if (enable_v2_sandbox && fd_mapping == -1) {
+    fprintf(stderr, "Must pass a valid file descriptor to --fd_mapping.\n");
+    return -1;

[Mark Mentovai, 2017/05/23 20:41:29]

-1’s not right here either.

[Greg K, 2017/05/23 23:31:29]

Done.

+  }
+
+  if (enable_v2_sandbox)
+    return SandboxExec(argc, argv, fd_mapping);
+
   const char* const rel_path =
       "../../../" PRODUCT_FULLNAME_STRING
       " Framework.framework/" PRODUCT_FULLNAME_STRING " Framework";
 #else
   const char* const rel_path =
       "../Versions/" CHROME_VERSION_STRING "/" PRODUCT_FULLNAME_STRING
       " Framework.framework/" PRODUCT_FULLNAME_STRING " Framework";
 #endif  // defined(HELPER_EXECUTABLE)
 
   uint32_t exec_path_size = 0;
   int rv = _NSGetExecutablePath(NULL, &exec_path_size);
   if (rv != -1) {
     fprintf(stderr, "_NSGetExecutablePath: get length failed\n");
     abort();
   }
 
-  char* exec_path = malloc(exec_path_size);
-  if (!exec_path) {
-    fprintf(stderr, "malloc %u: %s\n", exec_path_size, strerror(errno));
-    abort();
-  }
-
+  char* exec_path = new char[exec_path_size];
   rv = _NSGetExecutablePath(exec_path, &exec_path_size);
   if (rv != 0) {
     fprintf(stderr, "_NSGetExecutablePath: get path failed\n");
     abort();
   }
 
   // Slice off the last part of the main executable path, and append the
   // version framework information.
   const char* parent_dir = dirname(exec_path);
   if (!parent_dir) {
     fprintf(stderr, "dirname %s: %s\n", exec_path, strerror(errno));
     abort();
   }
-  free(exec_path);
+  delete[] exec_path;
 
   const size_t parent_path_len = strlen(parent_dir);
   const size_t rel_path_len = strlen(rel_path);
   // 2 accounts for a trailing NUL byte and the '/' in the middle of the paths.
   const size_t framework_path_size = parent_path_len + rel_path_len + 2;
-  char* framework_path = malloc(framework_path_size);
-  if (!framework_path) {
-    fprintf(stderr, "malloc %zu: %s\n", framework_path_size, strerror(errno));
-    abort();
-  }
+  char* framework_path = new char[framework_path_size];
   snprintf(framework_path, framework_path_size, "%s/%s", parent_dir, rel_path);
 
   void* library = dlopen(framework_path, RTLD_LAZY | RTLD_LOCAL | RTLD_FIRST);
   if (!library) {
     fprintf(stderr, "dlopen %s: %s\n", framework_path, dlerror());
     abort();
   }
-  free(framework_path);
+  delete[] framework_path;
 
-  const ChromeMainPtr chrome_main = dlsym(library, "ChromeMain");
+  const ChromeMainPtr chrome_main =
+      reinterpret_cast<ChromeMainPtr>(dlsym(library, "ChromeMain"));
   if (!chrome_main) {
     fprintf(stderr, "dlsym ChromeMain: %s\n", dlerror());
     abort();
   }
   rv = chrome_main(argc, argv);
 
   // exit, don't return from main, to avoid the apparent removal of main from

[Mark Mentovai, 2017/05/23 20:41:29]

Shouldn’t this apply to the other early returns in this function?

[Greg K, 2017/05/23 23:31:30]

Yes although all the early returns should actually just be aborts.

   // stack backtraces under tail call optimization.
   exit(rv);
 }