Bug 1723204: Update tests to prevent crashes r=ckerschb,jdescottes,robwu
authorTom Ritter <tom@mozilla.com>
Tue, 24 Aug 2021 14:57:44 +0000
changeset 589761 6614bd9a8b01e332338238de69eec04354945c59
parent 589760 0310d86a37b5a00d1561a186832604c35cdd40d9
child 589762 8472c4395d9148fa38c7359b4a3d56adb99a0858
push id38732
push usersmolnar@mozilla.com
push dateTue, 24 Aug 2021 21:59:48 +0000
treeherdermozilla-central@f31a22da9629 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb, jdescottes, robwu
bugs1723204
milestone93.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1723204: Update tests to prevent crashes r=ckerschb,jdescottes,robwu This is most commonly as a result of CU.evalInSandbox which allows an arbitrary filename but when omitted will default to the filename of the test, which is a filesystem path and thus is disallowed. Differential Revision: https://phabricator.services.mozilla.com/D122246
browser/components/migration/tests/marionette/test_refresh_firefox.py
browser/components/newtab/test/xpcshell/test_AboutHomeStartupCacheWorker.js
devtools/server/tests/xpcshell/xpcshell.ini
devtools/shared/heapsnapshot/tests/xpcshell/test_HeapSnapshot_deepStack_01.js
devtools/shared/heapsnapshot/tests/xpcshell/test_ReadHeapSnapshot_with_allocations.js
devtools/shared/tests/xpcshell/test_isSet.js
devtools/shared/webconsole/test/xpcshell/test_js_property_provider.js
dom/security/nsContentSecurityUtils.cpp
toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
--- a/browser/components/migration/tests/marionette/test_refresh_firefox.py
+++ b/browser/components/migration/tests/marionette/test_refresh_firefox.py
@@ -452,17 +452,19 @@ class TestFirefoxRefresh(MarionetteTestC
               content.document.getElementById("errorTryAgain").click();
             } else {
               content.window.addEventListener("load", function(event) {
                 content.document.getElementById("errorTryAgain").click();
               }, { once: true });
             }
           };
 
+          Services.prefs.setBoolPref("security.allow_parent_unrestricted_js_loads", true);
           mm.loadFrameScript("data:application/javascript,(" + fs.toString() + ")()", true);
+          Services.prefs.setBoolPref("security.allow_parent_unrestricted_js_loads", false);
         """  # NOQA: E501
         )
         self.assertSequenceEqual(tabURIs, self._expectedURLs)
 
     def checkFxA(self):
         result = self.runAsyncCode(
             """
           Cu.import("resource://gre/modules/FxAccountsStorage.jsm");
--- a/browser/components/newtab/test/xpcshell/test_AboutHomeStartupCacheWorker.js
+++ b/browser/components/newtab/test/xpcshell/test_AboutHomeStartupCacheWorker.js
@@ -136,16 +136,24 @@ add_task(async function setup() {
 });
 
 /**
  * Gets the Activity Stream Redux state from Activity Stream and sends it
  * into an instance of the cache worker to ensure that the resulting markup
  * and script makes sense.
  */
 add_task(async function test_cache_worker() {
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   let state = AboutNewTab.activityStream.store.getState();
 
   let cacheWorker = new BasePromiseWorker(CACHE_WORKER_URL);
   let { page, script } = await cacheWorker.post("construct", [state]);
   ok(!!page.length, "Got page content");
   ok(!!script.length, "Got script content");
 
   // The template strings should have been replaced.
--- a/devtools/server/tests/xpcshell/xpcshell.ini
+++ b/devtools/server/tests/xpcshell/xpcshell.ini
@@ -1,13 +1,17 @@
 [DEFAULT]
 tags = devtools
 head = head_dbg.js
 firefox-appdir = browser
 skip-if = toolkit == 'android'
+# While not every devtools test uses evalInSandbox over 80 do, so it's easier to
+# set the pref for all the tests here.
+prefs =
+  security.allow_parent_unrestricted_js_loads=true
 
 support-files =
   completions.js
   webextension-helpers.js
   source-map-data/sourcemapped.coffee
   source-map-data/sourcemapped.map
   post_init_global_actors.js
   post_init_target_scoped_actors.js
--- a/devtools/shared/heapsnapshot/tests/xpcshell/test_HeapSnapshot_deepStack_01.js
+++ b/devtools/shared/heapsnapshot/tests/xpcshell/test_HeapSnapshot_deepStack_01.js
@@ -5,16 +5,24 @@
 // Test that we can save a core dump with very deep allocation stacks and read
 // it back into a HeapSnapshot.
 
 function stackDepth(stack) {
   return stack ? 1 + stackDepth(stack.parent) : 0;
 }
 
 function run_test() {
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   // Create a Debugger observing a debuggee's allocations.
   const debuggee = new Cu.Sandbox(null);
   const dbg = new Debugger(debuggee);
   dbg.memory.trackingAllocationSites = true;
 
   // Allocate some objects in the debuggee that will have their allocation
   // stacks recorded by the Debugger.
 
--- a/devtools/shared/heapsnapshot/tests/xpcshell/test_ReadHeapSnapshot_with_allocations.js
+++ b/devtools/shared/heapsnapshot/tests/xpcshell/test_ReadHeapSnapshot_with_allocations.js
@@ -8,16 +8,24 @@
 if (typeof Debugger != "function") {
   const { addDebuggerToGlobal } = ChromeUtils.import(
     "resource://gre/modules/jsdebugger.jsm"
   );
   addDebuggerToGlobal(this);
 }
 
 function run_test() {
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   // Create a Debugger observing a debuggee's allocations.
   const debuggee = new Cu.Sandbox(null);
   const dbg = new Debugger(debuggee);
   dbg.memory.trackingAllocationSites = true;
 
   // Allocate some objects in the debuggee that will have their allocation
   // stacks recorded by the Debugger.
   debuggee.eval("this.objects = []");
--- a/devtools/shared/tests/xpcshell/test_isSet.js
+++ b/devtools/shared/tests/xpcshell/test_isSet.js
@@ -1,16 +1,24 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Test ThreadSafeDevToolsUtils.isSet
 
 function run_test() {
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   const { isSet } = DevToolsUtils;
 
   equal(isSet(new Set()), true);
   equal(isSet(new Map()), false);
   equal(isSet({}), false);
   equal(isSet("I swear I'm a Set"), false);
   equal(isSet(5), false);
 
--- a/devtools/shared/webconsole/test/xpcshell/test_js_property_provider.js
+++ b/devtools/shared/webconsole/test/xpcshell/test_js_property_provider.js
@@ -1,23 +1,32 @@
 // Any copyright is dedicated to the Public Domain.
 // http://creativecommons.org/publicdomain/zero/1.0/
 
 "use strict";
 const { require } = ChromeUtils.import("resource://devtools/shared/Loader.jsm");
+const Services = require("Services");
 const {
   FallibleJSPropertyProvider: JSPropertyProvider,
 } = require("devtools/shared/webconsole/js-property-provider");
 
 const { addDebuggerToGlobal } = ChromeUtils.import(
   "resource://gre/modules/jsdebugger.jsm"
 );
 addDebuggerToGlobal(this);
 
 function run_test() {
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   const testArray = `var testArray = [
     {propA: "A"},
     {
       propB: "B",
       propC: [
         "D"
       ]
     },
--- a/dom/security/nsContentSecurityUtils.cpp
+++ b/dom/security/nsContentSecurityUtils.cpp
@@ -463,30 +463,31 @@ FilenameTypeAndDetails nsContentSecurity
 #endif
 
   return FilenameTypeAndDetails(kOther, Nothing());
 }
 
 #ifdef NIGHTLY_BUILD
 // Crash String must be safe from a telemetry point of view.
 // This will be ensured when this function is used.
-void PossiblyCrash(const char* pref_suffix, const nsCString crash_string) {
+void PossiblyCrash(const char* aPrefSuffix, const char* aUnsafeCrashString,
+                   const nsCString& aSafeCrashString) {
   if (MOZ_UNLIKELY(!XRE_IsParentProcess())) {
     // We only crash in the parent (unfortunately) because it's
     // the only place we can be sure that our only-crash-once
     // pref-writing works.
     return;
   }
 
   nsCString previous_crashes("security.crash_tracking.");
-  previous_crashes.Append(pref_suffix);
+  previous_crashes.Append(aPrefSuffix);
   previous_crashes.Append(".prevCrashes");
 
   nsCString max_crashes("security.crash_tracking.");
-  max_crashes.Append(pref_suffix);
+  max_crashes.Append(aPrefSuffix);
   max_crashes.Append(".maxCrashes");
 
   int32_t numberOfPreviousCrashes = 0;
   numberOfPreviousCrashes = Preferences::GetInt(previous_crashes.get(), 0);
 
   int32_t maxAllowableCrashes = 0;
   maxAllowableCrashes = Preferences::GetInt(max_crashes.get(), 0);
 
@@ -505,17 +506,22 @@ void PossiblyCrash(const char* pref_suff
 
   if (!prefs->AllowOffMainThreadSave()) {
     // Do not crash if we can't save prefs off the main thread
     return;
   }
 
   rv = prefs->SavePrefFileBlocking();
   if (!NS_FAILED(rv)) {
-    MOZ_CRASH_UNSAFE_PRINTF("%s", nsContentSecurityUtils::SmartFormatCrashString(crash_string.get()));
+    // We can only use this in local builds where we don't send stuff up to the
+    // crash reporter because it has user private data.
+    // MOZ_CRASH_UNSAFE_PRINTF("%s",
+    //                        nsContentSecurityUtils::SmartFormatCrashString(aUnsafeCrashString));
+    MOZ_CRASH_UNSAFE_PRINTF(
+        "%s", nsContentSecurityUtils::SmartFormatCrashString(aSafeCrashString.get()));
   }
 }
 #endif
 
 class EvalUsageNotificationRunnable final : public Runnable {
  public:
   EvalUsageNotificationRunnable(bool aIsSystemPrincipal,
                                 NS_ConvertUTF8toUTF16& aFileNameA,
@@ -1221,20 +1227,20 @@ bool nsContentSecurityUtils::ValidateScr
                          extra);
 
 #ifdef NIGHTLY_BUILD
   // Cause a crash (if we've never crashed before and we can ensure we won't do
   // it again.)
   // The details in the second arg, passed to UNSAFE_PRINTF, are also included
   // in Event Telemetry and have received data review.
   if (fileNameTypeAndDetails.second.isSome()) {
-    PossiblyCrash("js_load_1",
+    PossiblyCrash("js_load_1", aFilename,
                   NS_ConvertUTF16toUTF8(fileNameTypeAndDetails.second.value()));
   } else {
-    PossiblyCrash("js_load_1", "(None)"_ns);
+    PossiblyCrash("js_load_1", aFilename, "(None)"_ns);
   }
 #endif
 
   // Presently we are not enforcing any restrictions for the script filename,
   // we're only reporting Telemetry. In the future we will assert in debug
   // builds and return false to prevent execution in non-debug builds.
   return true;
 }
--- a/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js
@@ -167,16 +167,26 @@ add_task(async function setup() {
     "1",
     "43"
   );
 
   ExtensionParent.apiManager.registerModules(MODULE_INFO);
 });
 
 add_task(async function test_persistent_events() {
+  // The blob:-URL registered above in MODULE_INFO gets loaded at
+  // https://searchfox.org/mozilla-central/rev/0fec57c05d3996cc00c55a66f20dd5793a9bfb5d/toolkit/components/extensions/ExtensionCommon.jsm#1649
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("security.allow_parent_unrestricted_js_loads");
+  });
+
   await AddonTestUtils.promiseStartupManager();
 
   let extension = ExtensionTestUtils.loadExtension({
     useAddonManager: "permanent",
     background() {
       let register1 = true,
         register2 = true;
       if (localStorage.getItem("skip1")) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
@@ -342,26 +342,34 @@ add_task(async function sendMessageRespo
   extension.sendMessage("callback-save");
 
   // Test returning a Promise that can never resolve.
   extension.sendMessage("promise-never");
 
   extension.sendMessage("ping");
   await extension.awaitMessage("pong");
 
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    true
+  );
   Services.ppmm.loadProcessScript("data:,Components.utils.forceGC()", false);
   await extension.awaitMessage("rejected");
 
   // Test returning `true` without holding the response handle.
   extension.sendMessage("callback-never");
 
   extension.sendMessage("ping");
   await extension.awaitMessage("pong");
 
   Services.ppmm.loadProcessScript("data:,Components.utils.forceGC()", false);
+  Services.prefs.setBoolPref(
+    "security.allow_parent_unrestricted_js_loads",
+    false
+  );
   await extension.awaitMessage("rejected");
 
   // Test that promises from long-running tasks didn't get GCd.
   extension.sendMessage("promise-resolve");
   await extension.awaitMessage("saved-resolve");
 
   extension.sendMessage("callback-call");
   await extension.awaitMessage("saved-respond");