[tor-commits] [tor-browser/tor-browser-60.2.0esr-8.5-1] Bug 26381: about:tor page does not load on first start on Windows

[gk at torproject.org]

commit 62001bb019dbf0956454f8fc550ccc1e81ee8d3c
Author: Richard Pospesel <richard at torproject.org>
Date:   Sat Sep 15 04:01:17 2018 +0000

    Bug 26381: about:tor page does not load on first start on Windows
    
    Child content processes require certain directories to be marked as
    readable or writeable when Sandboxing is enabled. The directories
    to be whitelisted are saved in static variables in
    sandboxBroker.cpp and are initialized in
    SandboxBroker::GeckoDependentInitialize(). Any child content process
    which is created before these directories are saved will be unable to
    read or write to them.
    
    The tor-launcher extension triggers the creation of a content process
    which hosts the tor network configuration settings window. This process
    is created before the whitelisted directories are saved.  The network
    settings process doesn't need access to these directories to function,
    but subsequent content processes which are created once the settings
    window exits do need these directories to function.  Sometimes, the
    creation of these subsequent processes is slow enough for the parent
    process to 'catch up' and create the whitelist resulting in the broken
    about:tor tab or broken white tab.
    
    A previous iteration of this patch moved the GeckoDependentInitialize()
    call directly above the call to DoStartup().  However, Mozilla dev Bob
    Owen objected to this since this places the call before various
    services are initialized which the SandboxBroker may depend on.  Some
    experimentation would seem to confirm his objections: placing the
    whitelist init just prior to DoStartup() results in an empty value for
    the profile directory which prevents child processes reading the chrome
    and extensions directory.
    
    This patch inserts the GeckoDependentInitialize() call into DoStartup()
    just after the profile directory is known and queryable by the
    SandboxBroker, and before the 'profile-after-change' notification is
    fired.  It also reverts the temp fix which reduced the sandbox level to
    2 on windows.
---
 browser/app/profile/000-tor-browser.js |  5 -----
 toolkit/xre/nsAppRunner.cpp            |  6 ------
 toolkit/xre/nsXREDirProvider.cpp       | 19 +++++++++++++++++++
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/browser/app/profile/000-tor-browser.js b/browser/app/profile/000-tor-browser.js
index bc7c4b05e3a1..38f72579760a 100644
--- a/browser/app/profile/000-tor-browser.js
+++ b/browser/app/profile/000-tor-browser.js
@@ -329,11 +329,6 @@ pref("browser.onboarding.newtour", "welcome,privacy,tor-network,circuit-display,
 pref("browser.onboarding.updatetour", "welcome,privacy,tor-network,circuit-display,security,expect-differences,onion-services");
 pref("browser.onboarding.skip-tour-button.hide", true);
 
-#ifdef XP_WIN
-// For now, reduce sandboxing level to 2 (see #26381).
-pref("security.sandbox.content.level", 2);
-#endif
-
 #ifdef TOR_BROWSER_VERSION
 #expand pref("torbrowser.version", __TOR_BROWSER_VERSION__);
 #endif
diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp
index 1000014aedd0..7889919ca677 100644
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -5249,12 +5249,6 @@ XREMain::XRE_mainRun()
     // We intentionally leak the string here since it is required by PR_SetEnv.
     PR_SetEnv(saved.release());
   }
-
-#if defined(MOZ_SANDBOX)
-  // Call SandboxBroker to initialize things that depend on Gecko machinery like
-  // the directory provider.
-  SandboxBroker::GeckoDependentInitialize();
-#endif
 #endif
 
   SaveStateForAppInitiatedRestart();
diff --git a/toolkit/xre/nsXREDirProvider.cpp b/toolkit/xre/nsXREDirProvider.cpp
index b54985292e81..72545d0eb422 100644
--- a/toolkit/xre/nsXREDirProvider.cpp
+++ b/toolkit/xre/nsXREDirProvider.cpp
@@ -66,6 +66,10 @@
 #include "UIKitDirProvider.h"
 #endif
 
+#if defined(MOZ_SANDBOX) && defined(XP_WIN)
+#include "sandboxBroker.h"
+#endif
+
 #if defined(MOZ_CONTENT_SANDBOX)
 #include "mozilla/SandboxSettings.h"
 #include "nsIUUIDGenerator.h"
@@ -1003,6 +1007,21 @@ nsXREDirProvider::DoStartup()
       policies->Observe(nullptr, "policies-startup", nullptr);
     }
 
+  #if defined(MOZ_SANDBOX) && defined(XP_WIN)
+    // Call SandboxBroker to initialize things that depend on Gecko machinery like
+    // the directory provider.
+
+    // We insert this initialization code here so that any child content processes spawned by
+    // extensions (such as tor-launcher launching the network configuration window) will have
+    // all the requisite directories white-listed for read/write access
+
+    // It's inserted here (rather than in XREMain::XRE_mainRun) because we need
+    // NS_APP_USER_PROFILE_50_DIR to be known
+
+    // See tor bug #26381 and mozilla bug #1485836
+    SandboxBroker::GeckoDependentInitialize();
+  #endif
+
     // Init the Extension Manager
     nsCOMPtr<nsIObserver> em = do_GetService("@mozilla.org/addons/integration;1");
     if (em) {