Bug 1363301 - Always provide live wrappers for ScriptSourceObjects. r=shu, r=bholley, a=jcristau FENNEC_55_0b3_BUILD1 FENNEC_55_0b3_RELEASE FIREFOX_55_0b3_BUILD1 FIREFOX_55_0b3_RELEASE
authorKris Maglione <maglione.k@gmail.com>
Wed, 14 Jun 2017 15:38:59 -0700
changeset 411660 be4a0ad5d6cab12fb89495d020532040b91be1f4
parent 411659 fd6fc586fbf3db466582cd907c6b466743582b15
child 411661 71dbd4d4e11e208109b1819724e360092d9a23b9
push id7430
push userryanvm@gmail.com
push dateMon, 19 Jun 2017 14:17:03 +0000
treeherdermozilla-beta@be4a0ad5d6ca [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, bholley, jcristau
bugs1363301
milestone55.0
Bug 1363301 - Always provide live wrappers for ScriptSourceObjects. r=shu, r=bholley, a=jcristau MozReview-Commit-ID: LTNN66FywU4
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/tests/unit/test_subScriptLoader.js
js/xpconnect/tests/unit/xpcshell.ini
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -570,16 +570,22 @@ JS_IsDeadWrapper(JSObject* obj)
 }
 
 JS_FRIEND_API(JSObject*)
 JS_NewDeadWrapper(JSContext* cx, JSObject* origObj)
 {
     return NewDeadProxyObject(cx, origObj);
 }
 
+JS_FRIEND_API(bool)
+JS_IsScriptSourceObject(JSObject* obj)
+{
+    return obj->is<ScriptSourceObject>();
+}
+
 void
 js::TraceWeakMaps(WeakMapTracer* trc)
 {
     WeakMapBase::traceAllMappings(trc);
     WatchpointMap::traceAll(trc);
 }
 
 extern JS_FRIEND_API(bool)
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -101,16 +101,22 @@ JS_IsDeadWrapper(JSObject* obj);
  * attempting to wrap objects from scopes which are already dead.
  *
  * If origObject is passed, it must be an proxy object, and will be
  * used to determine the characteristics of the new dead wrapper.
  */
 extern JS_FRIEND_API(JSObject*)
 JS_NewDeadWrapper(JSContext* cx, JSObject* origObject = nullptr);
 
+/**
+ * Determine whether the given object is a ScriptSourceObject.
+ */
+extern JS_FRIEND_API(bool)
+JS_IsScriptSourceObject(JSObject* obj);
+
 /*
  * Used by the cycle collector to trace through a shape or object group and
  * all cycle-participating data it reaches, using bounded stack space.
  */
 extern JS_FRIEND_API(void)
 JS_TraceShapeCycleCollectorChildren(JS::CallbackTracer* trc, JS::GCCellPtr shape);
 extern JS_FRIEND_API(void)
 JS_TraceObjectGroupCycleCollectorChildren(JS::CallbackTracer* trc, JS::GCCellPtr group);
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -213,16 +213,36 @@ EvalScript(JSContext* cx,
         nsresult rv = secman->GetSystemPrincipal(getter_AddRefs(principal));
         if (NS_FAILED(rv) || !principal) {
             ReportError(cx, LOAD_ERROR_NOPRINCIPALS, uri);
             return false;
         }
 
         nsCString uriStr;
         if (preloadCache && NS_SUCCEEDED(uri->GetSpec(uriStr))) {
+            // Note that, when called during startup, this will keep the
+            // original JSScript object alive for an indefinite amount of time.
+            // This has the side-effect of keeping the global that the script
+            // was compiled for alive, too.
+            //
+            // For most startups, the global in question will be the
+            // CompilationScope, since we pre-compile any scripts that were
+            // needed during the last startup in that scope. But for startups
+            // when a non-cached script is used (e.g., after add-on
+            // installation), this may be a Sandbox global, which may be
+            // nuked but held alive by the JSScript.
+            //
+            // In general, this isn't a problem, since add-on Sandboxes which
+            // use the script preloader are not destroyed until add-on shutdown,
+            // and when add-ons are uninstalled or upgraded, the preloader cache
+            // is immediately flushed after shutdown. But it's possible to
+            // disable and reenable an add-on without uninstalling it, leading
+            // to cached scripts being held alive, and tied to nuked Sandbox
+            // globals. Given the unusual circumstances required to trigger
+            // this, it's not a major concern. But it should be kept in mind.
             ScriptPreloader::GetSingleton().NoteScript(uriStr, cachePath, script);
         }
 
         if (startupCache) {
             JSAutoCompartment ac(cx, script);
             WriteCachedScript(StartupCache::GetSingleton(),
                               cachePath, cx, principal, script);
         }
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_subScriptLoader.js
@@ -0,0 +1,20 @@
+"use strict";
+
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+
+add_task(function* test_executeScriptAfterNuked() {
+  let scriptUrl = Services.io.newFileURI(do_get_file("file_simple_script.js")).spec;
+
+  // Load the script for the first time into a sandbox, and then nuke
+  // that sandbox.
+  let sandbox = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal());
+  Services.scriptloader.loadSubScript(scriptUrl, sandbox);
+  Cu.nukeSandbox(sandbox);
+
+  // Load the script again into a new sandbox, and make sure it
+  // succeeds.
+  sandbox = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal());
+  Services.scriptloader.loadSubScript(scriptUrl, sandbox);
+});
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -94,16 +94,17 @@ skip-if = toolkit == "android" # bug 134
 [test_tearoffs.js]
 [test_want_components.js]
 [test_components.js]
 [test_allowedDomains.js]
 [test_allowedDomainsXHR.js]
 [test_nuke_sandbox.js]
 [test_nuke_sandbox_event_listeners.js]
 [test_nuke_webextension_wrappers.js]
+[test_subScriptLoader.js]
 [test_rewrap_dead_wrapper.js]
 [test_sandbox_metadata.js]
 [test_exportFunction.js]
 [test_promise.js]
 [test_returncode.js]
 [test_textDecoder.js]
 [test_url.js]
 [test_URLSearchParams.js]
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -186,20 +186,24 @@ WrapperFactory::PrepareForWrapping(JSCon
     if (JS_IsDeadWrapper(obj)) {
         retObj.set(JS_NewDeadWrapper(cx, obj));
         return;
     }
 
     // If we've somehow gotten to this point after either the source or target
     // compartment has been nuked, return a DeadObjectProxy to prevent further
     // access.
+    // However, we always need to provide live wrappers for ScriptSourceObjects,
+    // since they're used for cross-compartment cloned scripts, and need to
+    // remain accessible even after the original compartment has been nuked.
     JSCompartment* origin = js::GetObjectCompartment(obj);
     JSCompartment* target = js::GetObjectCompartment(scope);
-    if (CompartmentPrivate::Get(origin)->wasNuked ||
-        CompartmentPrivate::Get(target)->wasNuked) {
+    if (!JS_IsScriptSourceObject(obj) &&
+        (CompartmentPrivate::Get(origin)->wasNuked ||
+         CompartmentPrivate::Get(target)->wasNuked)) {
         NS_WARNING("Trying to create a wrapper into or out of a nuked compartment");
 
         retObj.set(JS_NewDeadWrapper(cx));
         return;
     }
 
 
     // If we've got a WindowProxy, there's nothing special that needs to be
@@ -347,17 +351,18 @@ WrapperFactory::PrepareForWrapping(JSCon
     retObj.set(waive ? WaiveXray(cx, obj) : obj);
 }
 
 #ifdef DEBUG
 static void
 DEBUG_CheckUnwrapSafety(HandleObject obj, const js::Wrapper* handler,
                         JSCompartment* origin, JSCompartment* target)
 {
-    if (CompartmentPrivate::Get(origin)->wasNuked || CompartmentPrivate::Get(target)->wasNuked) {
+    if (!JS_IsScriptSourceObject(obj) &&
+        (CompartmentPrivate::Get(origin)->wasNuked || CompartmentPrivate::Get(target)->wasNuked)) {
         // If either compartment has already been nuked, we should have returned
         // a dead wrapper from our prewrap callback, and this function should
         // not be called.
         MOZ_ASSERT_UNREACHABLE("CheckUnwrapSafety called for a dead wrapper");
     } else if (AccessCheck::isChrome(target) || xpc::IsUniversalXPConnectEnabled(target)) {
         // If the caller is chrome (or effectively so), unwrap should always be allowed.
         MOZ_ASSERT(!handler->hasSecurityPolicy());
     } else if (CompartmentPrivate::Get(origin)->forcePermissiveCOWs) {