Bug 1322273: Return DeadObjectProxy when wrapping for nuked compartment. r=bholley draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 22 Feb 2017 19:11:52 -0800
changeset 488798 46a2f4c2a23a56aaf7bbbc8f312ccbf374b5d60a
parent 488797 69777629c4162cd276b4ba387209e2a1fad3dc2d
child 546838 48a05f580bc073652bc54dd5c54e8ac204ff493d
push id46645
push usermaglione.k@gmail.com
push dateThu, 23 Feb 2017 19:41:52 +0000
reviewersbholley
bugs1322273
milestone54.0a1
Bug 1322273: Return DeadObjectProxy when wrapping for nuked compartment. r=bholley MozReview-Commit-ID: V07P0eZvKO
js/xpconnect/tests/unit/test_nuke_sandbox.js
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/xpconnect/tests/unit/test_nuke_sandbox.js
+++ b/js/xpconnect/tests/unit/test_nuke_sandbox.js
@@ -1,29 +1,54 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* See https://bugzilla.mozilla.org/show_bug.cgi?id=769273 */
 
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+const global = this;
+
 function run_test()
 {
-  var sb = Components.utils.Sandbox("http://www.blah.com");
+  var ifacePointer = Cc["@mozilla.org/supports-interface-pointer;1"]
+      .createInstance(Ci.nsISupportsInterfacePointer);
+
+  var sb = Cu.Sandbox(global);
   sb.prop = "prop"
-  var refToObjFromSb = Components.utils.evalInSandbox("var a = {prop2:'prop2'}; a", sb); 
-  Components.utils.nukeSandbox(sb);
-  do_check_true(Components.utils.isDeadWrapper(sb), "sb should be dead");
+  sb.ifacePointer = ifacePointer
+
+  var refToObjFromSb = Cu.evalInSandbox(`
+    Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+    ifacePointer.data = {
+      QueryInterface: XPCOMUtils.generateQI([]),
+      wrappedJSObject: {foo: "bar"},
+    };
+
+    var a = {prop2:'prop2'};
+    a
+  `, sb);
+
+  equal(ifacePointer.data.wrappedJSObject.foo, "bar",
+        "Got expected wrapper into sandbox")
+
+  Cu.nukeSandbox(sb);
+  ok(Cu.isDeadWrapper(sb), "sb should be dead");
+  ok(Cu.isDeadWrapper(ifacePointer.data.wrappedJSObject),
+     "Wrapper retrieved via XPConnect should be dead");
+
   try{
     sb.prop;
     do_check_true(false);
   } catch (e) {
     do_check_true(e.toString().indexOf("can't access dead object") > -1);
   }
-  
+
   Components.utils.isDeadWrapper(refToObjFromSb, "ref to object from sb should be dead");
   try{
     refToObjFromSb.prop2;
     do_check_true(false);
   } catch (e) {
     do_check_true(e.toString().indexOf("can't access dead object") > -1);
   }
-  
 }
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -176,16 +176,34 @@ WrapperFactory::PrepareForWrapping(JSCon
         }
         MOZ_ASSERT(js::IsWindowProxy(obj));
         // We crossed a compartment boundary there, so may now have a gray
         // object.  This function is not allowed to return gray objects, so
         // don't do that.
         ExposeObjectToActiveJS(obj);
     }
 
+    // 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.
+    JSCompartment* origin = js::GetObjectCompartment(obj);
+    JSCompartment* target = js::GetObjectCompartment(scope);
+    if (CompartmentPrivate::Get(origin)->wasNuked ||
+        CompartmentPrivate::Get(target)->wasNuked) {
+        NS_WARNING("Trying to create a wrapper into or out of a nuked compartment");
+
+        RootedObject ccw(cx, Wrapper::New(cx, obj, &CrossCompartmentWrapper::singleton));
+
+        NukeCrossCompartmentWrapper(cx, ccw);
+
+        retObj.set(ccw);
+        return;
+    }
+
+
     // If we've got a WindowProxy, there's nothing special that needs to be
     // done here, and we can move on to the next phase of wrapping. We handle
     // this case first to allow us to assert against wrappers below.
     if (js::IsWindowProxy(obj)) {
         retObj.set(waive ? WaiveXray(cx, obj) : obj);
         return;
     }
 
@@ -327,18 +345,20 @@ WrapperFactory::PrepareForWrapping(JSCon
 }
 
 #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 either compartment has already been nuked, we should have an opaque wrapper.
-        MOZ_ASSERT(handler->hasSecurityPolicy());
+        // 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) {
         // Similarly, if this is a privileged scope that has opted to make itself
         // accessible to the world (allowed only during automation), unwrap should
         // be allowed.
         MOZ_ASSERT(!handler->hasSecurityPolicy());
@@ -452,31 +472,19 @@ WrapperFactory::Rewrap(JSContext* cx, Ha
     bool sameOrigin = targetSubsumesOrigin && originSubsumesTarget;
 
     const Wrapper* wrapper;
 
     //
     // First, handle the special cases.
     //
 
-    // If we've somehow gotten to this point after either the source or target
-    // compartment has been nuked, return an opaque wrapper to prevent further
-    // access.
-    // Ideally, we should return a DeadProxyObject instead of a wrapper in this
-    // case (bug 1322273).
-    if (CompartmentPrivate::Get(origin)->wasNuked ||
-        CompartmentPrivate::Get(target)->wasNuked) {
-        NS_WARNING("Trying to create a wrapper into or out of a nuked compartment");
-
-        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>::singleton;
-    }
-
     // If UniversalXPConnect is enabled, this is just some dumb mochitest. Use
     // a vanilla CCW.
-    else if (xpc::IsUniversalXPConnectEnabled(target)) {
+    if (xpc::IsUniversalXPConnectEnabled(target)) {
         CrashIfNotInAutomation();
         wrapper = &CrossCompartmentWrapper::singleton;
     }
 
     // Let the SpecialPowers scope make its stuff easily accessible to content.
     else if (CompartmentPrivate::Get(origin)->forcePermissiveCOWs) {
         CrashIfNotInAutomation();
         wrapper = &CrossCompartmentWrapper::singleton;