Bug 1405088 - Part 1 - remove file-write permissions from macOS content temporary directory; r?haik draft
authorAlex Gaynor <agaynor@mozilla.com>
Mon, 02 Oct 2017 10:48:01 -0400
changeset 754533 b459a3e390fe2b71760113aded7907d96fd99605
parent 754399 38b3c1d03a594664c6b32c35533734283c258f43
child 754534 2e480288bdb5a455e5b6c185365c7d8ffb096a71
push id98908
push userbmo:agaynor@mozilla.com
push dateTue, 13 Feb 2018 18:28:35 +0000
reviewershaik
bugs1405088
milestone60.0a1
Bug 1405088 - Part 1 - remove file-write permissions from macOS content temporary directory; r?haik With this change, the macOS content sandbox has no ability to create files anywhere on disk (in release builds). If the content process needs a file to write to, it needs to obtain a file descriptor from the parent process. MozReview-Commit-ID: 7LoG1PW0UDR
dom/ipc/ContentChild.cpp
security/sandbox/mac/Sandbox.h
security/sandbox/mac/Sandbox.mm
security/sandbox/mac/SandboxPolicies.h
security/sandbox/test/browser_content_sandbox_fs.js
security/sandbox/test/browser_content_sandbox_syscalls.js
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1538,35 +1538,19 @@ StartMacOSContentSandbox()
     CGSShutdownServerConnections();
   }
 
   nsAutoCString appPath, appBinaryPath, appDir;
   if (!GetAppPaths(appPath, appBinaryPath, appDir)) {
     MOZ_CRASH("Error resolving child process path");
   }
 
-  // During sandboxed content process startup, before reaching
-  // this point, NS_OS_TEMP_DIR is modified to refer to a sandbox-
-  // writable temporary directory
-  nsCOMPtr<nsIFile> tempDir;
-  nsresult rv = nsDirectoryService::gService->Get(NS_OS_TEMP_DIR,
-      NS_GET_IID(nsIFile), getter_AddRefs(tempDir));
-  if (NS_FAILED(rv)) {
-    MOZ_CRASH("Failed to get NS_OS_TEMP_DIR");
-  }
-
-  nsAutoCString tempDirPath;
-  tempDir->Normalize();
-  rv = tempDir->GetNativePath(tempDirPath);
-  if (NS_FAILED(rv)) {
-    MOZ_CRASH("Failed to get NS_OS_TEMP_DIR path");
-  }
-
   ContentChild* cc = ContentChild::GetSingleton();
 
+  nsresult rv;
   nsCOMPtr<nsIFile> profileDir;
   cc->GetProfileDir(getter_AddRefs(profileDir));
   nsCString profileDirPath;
   if (profileDir) {
     profileDir->Normalize();
     rv = profileDir->GetNativePath(profileDirPath);
     if (NS_FAILED(rv) || profileDirPath.IsEmpty()) {
       MOZ_CRASH("Failed to get profile path");
@@ -1579,17 +1563,16 @@ StartMacOSContentSandbox()
   info.type = MacSandboxType_Content;
   info.level = sandboxLevel;
   info.hasFilePrivileges = isFileProcess;
   info.shouldLog = Preferences::GetBool("security.sandbox.logging.enabled") ||
                    PR_GetEnv("MOZ_SANDBOX_LOGGING");
   info.appPath.assign(appPath.get());
   info.appBinaryPath.assign(appBinaryPath.get());
   info.appDir.assign(appDir.get());
-  info.appTempDir.assign(tempDirPath.get());
   info.hasAudio = !Preferences::GetBool("media.cubeb.sandbox");
 
   // These paths are used to whitelist certain directories used by the testing
   // system. They should not be considered a public API, and are only intended
   // for use in automation.
   nsAutoCString testingReadPath1;
   Preferences::GetCString("security.sandbox.content.mac.testing_read_path1",
                           testingReadPath1);
--- a/security/sandbox/mac/Sandbox.h
+++ b/security/sandbox/mac/Sandbox.h
@@ -47,17 +47,16 @@ typedef struct _MacSandboxInfo {
   int32_t level;
   bool hasFilePrivileges;
   bool hasSandboxedProfile;
   bool hasAudio;
   MacSandboxPluginInfo pluginInfo;
   std::string appPath;
   std::string appBinaryPath;
   std::string appDir;
-  std::string appTempDir;
   std::string profileDir;
   std::string debugWriteDir;
 
   std::string testingReadPath1;
   std::string testingReadPath2;
   std::string testingReadPath3;
   std::string testingReadPath4;
 
--- a/security/sandbox/mac/Sandbox.mm
+++ b/security/sandbox/mac/Sandbox.mm
@@ -160,18 +160,16 @@ bool StartMacSandbox(MacSandboxInfo cons
       params.push_back("MAC_OS_MINOR");
       params.push_back(macOSMinor.c_str());
       params.push_back("APP_PATH");
       params.push_back(aInfo.appPath.c_str());
       params.push_back("APP_BINARY_PATH");
       params.push_back(aInfo.appBinaryPath.c_str());
       params.push_back("APP_DIR");
       params.push_back(aInfo.appDir.c_str());
-      params.push_back("APP_TEMP_DIR");
-      params.push_back(aInfo.appTempDir.c_str());
       params.push_back("PROFILE_DIR");
       params.push_back(aInfo.profileDir.c_str());
       params.push_back("HOME_PATH");
       params.push_back(getenv("HOME"));
       params.push_back("HAS_SANDBOXED_PROFILE");
       params.push_back(aInfo.hasSandboxedProfile ? "TRUE" : "FALSE");
       if (!aInfo.testingReadPath1.empty()) {
         params.push_back("TESTING_READ_PATH1");
--- a/security/sandbox/mac/SandboxPolicies.h
+++ b/security/sandbox/mac/SandboxPolicies.h
@@ -47,17 +47,16 @@ static const char contentSandboxRules[] 
   (define should-log (param "SHOULD_LOG"))
   (define sandbox-level-1 (param "SANDBOX_LEVEL_1"))
   (define sandbox-level-2 (param "SANDBOX_LEVEL_2"))
   (define sandbox-level-3 (param "SANDBOX_LEVEL_3"))
   (define macosMinorVersion (string->number (param "MAC_OS_MINOR")))
   (define appPath (param "APP_PATH"))
   (define appBinaryPath (param "APP_BINARY_PATH"))
   (define appdir-path (param "APP_DIR"))
-  (define appTempDir (param "APP_TEMP_DIR"))
   (define hasProfileDir (param "HAS_SANDBOXED_PROFILE"))
   (define profileDir (param "PROFILE_DIR"))
   (define home-path (param "HOME_PATH"))
   (define debugWriteDir (param "DEBUG_WRITE_DIR"))
   (define testingReadPath1 (param "TESTING_READ_PATH1"))
   (define testingReadPath2 (param "TESTING_READ_PATH2"))
   (define testingReadPath3 (param "TESTING_READ_PATH3"))
   (define testingReadPath4 (param "TESTING_READ_PATH4"))
@@ -328,24 +327,16 @@ static const char contentSandboxRules[] 
       (iokit-user-client-class "AGPMClient")
       (iokit-user-client-class "AppleGraphicsControlClient"))
 
 ; bug 1153809
   (allow iokit-open
       (iokit-user-client-class "NVDVDContextTesla")
       (iokit-user-client-class "Gen6DVDContext"))
 
-  ; bug 1237847
-  (allow file-read* file-write-data
-    (subpath appTempDir))
-  (allow file-write-create
-    (require-all
-      (subpath appTempDir)
-      (vnode-type REGULAR-FILE)))
-
   ; Fonts
   (allow file-read*
     (subpath "/Library/Fonts")
     (subpath "/Library/Application Support/Apple/Fonts")
     (home-subpath "/Library/Fonts")
     ; Allow read access to paths allowed via sandbox extensions.
     ; This is needed for fonts in non-standard locations normally
     ; due to third party font managers. The extensions are
--- a/security/sandbox/test/browser_content_sandbox_fs.js
+++ b/security/sandbox/test/browser_content_sandbox_fs.js
@@ -220,23 +220,28 @@ async function createFileInHome() {
   let fileCreated = await ContentTask.spawn(browser, path, createFile);
   ok(fileCreated == false, "creating a file in home dir is not permitted");
   if (fileCreated == true) {
     // content process successfully created the file, now remove it
     homeFile.remove(false);
   }
 }
 
-// Test if the content process can create a temp file, should pass. Also test
-// that the content process cannot create symlinks or delete files.
+// Test if the content process can create a temp file, this is disallowed on
+// macOS but allowed everywhere else. Also test that the content process cannot
+// create symlinks or delete files.
 async function createTempFile() {
   let browser = gBrowser.selectedBrowser;
   let path = fileInTempDir().path;
   let fileCreated = await ContentTask.spawn(browser, path, createFile);
-  ok(fileCreated == true, "creating a file in content temp is permitted");
+  if (isMac()) {
+    ok(fileCreated == false, "creating a file in content temp is not permitted");
+  } else {
+    ok(fileCreated == true, "creating a file in content temp is permitted");
+  }
   // now delete the file
   let fileDeleted = await ContentTask.spawn(browser, path, deleteFile);
   if (isMac()) {
     // On macOS we do not allow file deletion - it is not needed by the content
     // process itself, and macOS uses a different permission to control access
     // so revoking it is easy.
     ok(fileDeleted == false,
        "deleting a file in content temp is not permitted");
--- a/security/sandbox/test/browser_content_sandbox_syscalls.js
+++ b/security/sandbox/test/browser_content_sandbox_syscalls.js
@@ -189,22 +189,27 @@ add_task(async function() {
     let path = fileInHomeDir().path;
     let flags = openWriteCreateFlags();
     let fd = await ContentTask.spawn(browser, {lib, path, flags}, callOpen);
     ok(fd < 0, "opening a file for writing in home is not permitted");
   }
 
   // use open syscall
   if (isLinux() || isMac()) {
-    // open a file for writing in the content temp dir, this should work
-    // and the open handler in the content process closes the file for us
+    // open a file for writing in the content temp dir, this should fail on
+    // macOS and work on Linux. The open handler in the content process closes
+    // the file for us
     let path = fileInTempDir().path;
     let flags = openWriteCreateFlags();
     let fd = await ContentTask.spawn(browser, {lib, path, flags}, callOpen);
-    ok(fd >= 0, "opening a file for writing in content temp is permitted");
+    if (isMac()) {
+      ok(fd === -1, "opening a file for writing in content temp is not permitted");
+    } else {
+      ok(fd >= 0, "opening a file for writing in content temp is permitted");
+    }
   }
 
   // use fork syscall
   if (isLinux() || isMac()) {
     let rv = await ContentTask.spawn(browser, {lib}, callFork);
     ok(rv == -1, "calling fork is not permitted");
   }