Bug 1354733: Part 2 - Never rewrap dead wrappers. r=bholley draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 20 May 2017 11:51:05 -0700
changeset 582004 e2156ebd591b4a08deda198a6d136bdf1ccda965
parent 582003 ec4fdfe91092efe4cb5144987d48e8e6741d20e0
child 629645 d2ec61f15c91456202643da0c7f71b228a93ebef
push id59946
push usermaglione.k@gmail.com
push dateSat, 20 May 2017 19:00:47 +0000
reviewersbholley
bugs1354733
milestone55.0a1
Bug 1354733: Part 2 - Never rewrap dead wrappers. r=bholley MozReview-Commit-ID: 2oSGtKe9pkI
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/tests/unit/test_rewrap_dead_wrapper.js
js/xpconnect/tests/unit/xpcshell.ini
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -2836,20 +2836,21 @@ nsXPCComponents_Utils::MakeObjectPropsNo
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::IsDeadWrapper(HandleValue obj, bool* out)
 {
     *out = false;
     if (obj.isPrimitive())
         return NS_ERROR_INVALID_ARG;
 
-    // Make sure to unwrap first. Once a proxy is nuked, it ceases to be a
-    // wrapper, meaning that, if passed to another compartment, we'll generate
-    // a CCW for it. Make sure that IsDeadWrapper sees through the confusion.
-    *out = JS_IsDeadWrapper(js::CheckedUnwrap(&obj.toObject()));
+    // We should never have cross-compartment wrappers for dead wrappers.
+    MOZ_ASSERT_IF(js::IsCrossCompartmentWrapper(obj),
+                  !JS_IsDeadWrapper(js::UncheckedUnwrap(&obj.toObject())));
+
+    *out = JS_IsDeadWrapper(&obj.toObject());
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::IsCrossProcessWrapper(HandleValue obj, bool* out)
 {
     *out = false;
     if (obj.isPrimitive())
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_rewrap_dead_wrapper.js
@@ -0,0 +1,33 @@
+/* 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=1354733 */
+
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+const global = this;
+
+function run_test()
+{
+  var sb = Cu.Sandbox(global);
+  let obj = new sb.Object();
+  Cu.nukeSandbox(sb);
+
+  ok(Cu.isDeadWrapper(obj), "object should be a dead wrapper");
+
+  // Create a new sandbox to wrap objects for.
+
+  var sb = Cu.Sandbox(global);
+  Cu.evalInSandbox(function echo(val) { return val; },
+                   sb);
+
+  let echoed = sb.echo(obj);
+  ok(Cu.isDeadWrapper(echoed), "Rewrapped object should be a dead wrapper");
+  ok(echoed !== obj, "Rewrapped object should be a new dead wrapper");
+
+  ok(obj === obj, "Dead wrapper object should be equal to itself");
+
+  let liveObj = {};
+  ok(liveObj === sb.echo(liveObj), "Rewrapped live object should be equal to itself");
+}
--- 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_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]
 [test_fileReader.js]
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -176,16 +176,23 @@ 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 the object is a dead wrapper, return a new dead wrapper rather than
+    // trying to wrap it for a different compartment.
+    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.
     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");