Bug 1386558 - Check sandboxing level 2 after permissions are available. r=jld draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Thu, 03 Aug 2017 12:31:37 +0200
changeset 620406 1c21f21d04cddd6c00e5f495c6686c671aa9cac1
parent 620405 71228e37d789ff5f8846099bb71cafa426d263e9
child 640675 de511b99df0c11c39eff873fcbc799e3fb692aff
push id72015
push usergpascutto@mozilla.com
push dateThu, 03 Aug 2017 10:32:22 +0000
reviewersjld
bugs1386558
milestone57.0a1
Bug 1386558 - Check sandboxing level 2 after permissions are available. r=jld MozReview-Commit-ID: 9Pqwk45pJbe
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -209,17 +209,19 @@ SandboxBroker::Policy::AddDir(int aPerms
     path.Append('/');
   }
   Policy::AddPrefixInternal(aPerms, path);
 
   // Add a path permission on the dir itself so it can
   // be opened. We're guaranteed to have a trailing / now,
   // so just cut that.
   path.Truncate(path.Length() - 1);
-  Policy::AddPath(aPerms, path.get(), AddAlways);
+  if (!path.IsEmpty()) {
+    Policy::AddPath(aPerms, path.get(), AddAlways);
+  }
 }
 
 void
 SandboxBroker::Policy::AddPrefix(int aPerms, const char* aPath)
 {
   Policy::AddPrefixInternal(aPerms, nsDependentCString(aPath));
 }
 
--- a/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
+++ b/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
@@ -83,22 +83,16 @@ SandboxBrokerPolicyFactory::SandboxBroke
   if (const auto userDir = g_get_user_runtime_dir()) {
     // The leaf filename is "user" by default, but is configurable.
     nsPrintfCString shmPath("%s/dconf/", userDir);
     policy->AddPrefix(rdwrcr, shmPath.get());
   }
 #endif
 
   // Read permissions
-  // No read blocking at level 2 and below
-  if (Preferences::GetInt("security.sandbox.content.level") <= 2) {
-    policy->AddDir(rdonly, "/");
-    mCommonContentPolicy.reset(policy);
-    return;
-  }
   policy->AddPath(rdonly, "/dev/urandom");
   policy->AddPath(rdonly, "/proc/cpuinfo");
   policy->AddPath(rdonly, "/proc/meminfo");
   policy->AddDir(rdonly, "/lib");
   policy->AddDir(rdonly, "/etc");
   policy->AddDir(rdonly, "/usr/share");
   policy->AddDir(rdonly, "/usr/local/share");
   policy->AddDir(rdonly, "/usr/lib");
@@ -224,39 +218,47 @@ SandboxBrokerPolicyFactory::GetContentPo
   if (GetEffectiveContentSandboxLevel() <= 1) {
     return nullptr;
   }
 
   MOZ_ASSERT(mCommonContentPolicy);
   UniquePtr<SandboxBroker::Policy>
     policy(new SandboxBroker::Policy(*mCommonContentPolicy));
 
+  // Read any extra paths that will get write permissions,
+  // configured by the user or distro
+  AddDynamicPathList(policy.get(),
+                     "security.sandbox.content.write_path_whitelist",
+                     rdwr);
+
+  // No read blocking at level 2 and below.
+  // file:// processes also get global read permissions
+  // This requires accessing user preferences so we can only do it now.
+  // Our constructor is initialized before user preferences are read in.
+  if (GetEffectiveContentSandboxLevel() <= 2 || aFileProcess) {
+    policy->AddDir(rdonly, "/");
+    return policy;
+  }
+
+  // Read permissions only from here on!
+  // Whitelisted for reading by the user/distro
+  AddDynamicPathList(policy.get(),
+                    "security.sandbox.content.read_path_whitelist",
+                    rdonly);
+
   // Bug 1198550: the profiler's replacement for dl_iterate_phdr
   policy->AddPath(rdonly, nsPrintfCString("/proc/%d/maps", aPid).get());
 
   // Bug 1198552: memory reporting.
   policy->AddPath(rdonly, nsPrintfCString("/proc/%d/statm", aPid).get());
   policy->AddPath(rdonly, nsPrintfCString("/proc/%d/smaps", aPid).get());
-  // Now read any extra paths, this requires accessing user preferences
-  // so we can only do it now. Our constructor is initialized before
-  // user preferences are read in.
-  AddDynamicPathList(policy.get(),
-                     "security.sandbox.content.read_path_whitelist",
-                     rdonly);
-  AddDynamicPathList(policy.get(),
-                     "security.sandbox.content.write_path_whitelist",
-                     rdwr);
 
-  // file:// processes get global read permissions
-  if (aFileProcess) {
-    policy->AddDir(rdonly, "/");
-  }
-
-  // userContent.css sits in the profile, which is normally blocked
-  // and we can't get the profile dir earlier
+  // userContent.css and the extensions dir sit in the profile, which is
+  // normally blocked and we can't get the profile dir earlier in startup,
+  // so this must happen here.
   nsCOMPtr<nsIFile> profileDir;
   nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                        getter_AddRefs(profileDir));
   if (NS_SUCCEEDED(rv)) {
       nsCOMPtr<nsIFile> workDir;
       rv = profileDir->Clone(getter_AddRefs(workDir));
       if (NS_SUCCEEDED(rv)) {
         rv = workDir->AppendNative(NS_LITERAL_CSTRING("chrome"));