Bug 1516237 - Fix FixWaiverAfterTransplant to nuke CCWs for oldWaiver in the new compartment. r=bholley
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 03 Jan 2019 09:04:02 +0000
changeset 452381 c5d46599eb99b0edacc1b29b66814189a0ab8423
parent 452380 a29627aa1ef5977bff1e93dcd150a274cfe79e15
child 452382 ea7f319717df7ae012cb57b70859fa5531ef5a0b
push id35304
push userdvarga@mozilla.com
push dateThu, 03 Jan 2019 16:24:35 +0000
treeherdermozilla-central@5b837856dca7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1516237
milestone66.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 1516237 - Fix FixWaiverAfterTransplant to nuke CCWs for oldWaiver in the new compartment. r=bholley This case can come up with same-compartment realms. Keeping these CCWs would confuse RemapWrapper because it'd be called with the CCW and target in the same compartment. Differential Revision: https://phabricator.services.mozilla.com/D15491
js/public/Wrapper.h
js/src/proxy/CrossCompartmentWrapper.cpp
js/xpconnect/tests/chrome/chrome.ini
js/xpconnect/tests/chrome/test_bug1516237.html
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/public/Wrapper.h
+++ b/js/public/Wrapper.h
@@ -419,16 +419,21 @@ JS_FRIEND_API JSObject* UnwrapOneChecked
 // the GC or off the main thread.
 JS_FRIEND_API JSObject* UncheckedUnwrapWithoutExpose(JSObject* obj);
 
 void ReportAccessDenied(JSContext* cx);
 
 JS_FRIEND_API void NukeCrossCompartmentWrapper(JSContext* cx,
                                                JSObject* wrapper);
 
+// If a cross-compartment wrapper source => target exists, nuke it.
+JS_FRIEND_API void NukeCrossCompartmentWrapperIfExists(JSContext* cx,
+                                                       JS::Compartment* source,
+                                                       JSObject* target);
+
 void RemapWrapper(JSContext* cx, JSObject* wobj, JSObject* newTarget);
 
 JS_FRIEND_API bool RemapAllWrappersForObject(JSContext* cx, JSObject* oldTarget,
                                              JSObject* newTarget);
 
 // API to recompute all cross-compartment wrappers whose source and target
 // match the given filters.
 JS_FRIEND_API bool RecomputeWrappers(JSContext* cx,
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -455,16 +455,27 @@ JS_FRIEND_API void js::NukeCrossCompartm
   JS::Compartment* comp = wrapper->compartment();
   auto ptr = comp->lookupWrapper(Wrapper::wrappedObject(wrapper));
   if (ptr) {
     comp->removeWrapper(ptr);
   }
   NukeRemovedCrossCompartmentWrapper(cx, wrapper);
 }
 
+JS_FRIEND_API void js::NukeCrossCompartmentWrapperIfExists(
+    JSContext* cx, JS::Compartment* source, JSObject* target) {
+  MOZ_ASSERT(source != target->compartment());
+  MOZ_ASSERT(!target->is<CrossCompartmentWrapperObject>());
+  auto ptr = source->lookupWrapper(target);
+  if (ptr) {
+    JSObject* wrapper = &ptr->value().get().toObject();
+    NukeCrossCompartmentWrapper(cx, wrapper);
+  }
+}
+
 // Returns true iff all realms in the compartment have been nuked.
 static bool NukedAllRealms(JS::Compartment* comp) {
   for (RealmsInCompartmentIter realm(comp); !realm.done(); realm.next()) {
     if (!realm->nukedIncomingWrappers) {
       return false;
     }
   }
   return true;
@@ -599,16 +610,17 @@ void js::RemapWrapper(JSContext* cx, JSO
   MOZ_ASSERT(wobj->is<CrossCompartmentWrapperObject>());
   MOZ_ASSERT(!newTarget->is<CrossCompartmentWrapperObject>());
   JSObject* origTarget = Wrapper::wrappedObject(wobj);
   MOZ_ASSERT(origTarget);
   MOZ_ASSERT(!JS_IsDeadWrapper(origTarget),
              "We don't want a dead proxy in the wrapper map");
   Value origv = ObjectValue(*origTarget);
   JS::Compartment* wcompartment = wobj->compartment();
+  MOZ_ASSERT(wcompartment != newTarget->compartment());
 
   AutoDisableProxyCheck adpc;
 
   // If we're mapping to a different target (as opposed to just recomputing
   // for the same target), we must not have an existing wrapper for the new
   // target, otherwise this will break.
   MOZ_ASSERT_IF(origTarget != newTarget,
                 !wcompartment->lookupWrapper(ObjectValue(*newTarget)));
--- a/js/xpconnect/tests/chrome/chrome.ini
+++ b/js/xpconnect/tests/chrome/chrome.ini
@@ -85,16 +85,17 @@ skip-if = os == 'win' || os == 'mac' || 
 [test_bug1065185.html]
 [test_bug1074863.html]
 [test_bug1092477.xul]
 [test_bug1124898.html]
 [test_bug1126911.html]
 [test_bug1281071.xul]
 [test_bug1390159.xul]
 [test_bug1430164.html]
+[test_bug1516237.html]
 [test_chrometoSource.xul]
 [test_cloneInto.xul]
 [test_cows.xul]
 [test_discardSystemSource.xul]
 [test_documentdomain.xul]
 [test_doublewrappedcompartments.xul]
 [test_evalInSandbox.xul]
 [test_evalInWindow.xul]
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug1516237.html
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1516237
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1516237</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://global/skin"/>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+/** Test for Bug 1516237 **/
+ChromeUtils.import("resource://testing-common/TestUtils.jsm");
+
+function go() {
+    SimpleTest.waitForExplicitFinish();
+
+    var frame = $('subframe');
+    frame.onload = null;
+
+    // Get a CCW wrapping an Xray waiver wrapping a WindowProxy.
+    var w = frame.contentWindow;
+    var ccwToWaiver = w.wrappedJSObject;
+    ccwToWaiver.testProp = 1;
+    is(ccwToWaiver.testProp, 1, "Can set properties on content window");
+
+    // Load a chrome page in the content frame. This might create a realm in
+    // the current chrome compartment - in that case we nuke the CCW to the
+    // waiver.
+    frame.onload = function() {
+        var sameCompartment =
+            Cu.getClassName(w.Math, /* unwrap = */ false) === "Math";
+        is(sameCompartment,
+           Cu.isDeadWrapper(ccwToWaiver),
+           "Nuked CCW if same-compartment window");
+        SimpleTest.finish();
+    };
+    frame.src = "file_empty.html";
+}
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1516237">Mozilla Bug 1516237</a>
+
+<iframe id="subframe" src="http://example.org/tests/js/xpconnect/tests/mochitest/file_empty.html" onload="go()"></iframe>
+
+</body>
+</html>
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -601,16 +601,32 @@ bool WrapperFactory::WaiveXrayAndWrap(JS
  * waivers get fixed up properly.
  */
 
 static bool FixWaiverAfterTransplant(JSContext* cx, HandleObject oldWaiver,
                                      HandleObject newobj) {
   MOZ_ASSERT(Wrapper::wrapperHandler(oldWaiver) == &XrayWaiver);
   MOZ_ASSERT(!js::IsCrossCompartmentWrapper(newobj));
 
+  // If the new compartment has a CCW for oldWaiver, nuke this CCW. This
+  // prevents confusing RemapAllWrappersForObject: it would call RemapWrapper
+  // with two same-compartment objects (the CCW and the new waiver).
+  //
+  // This can happen when loading a chrome page in a content frame and there
+  // exists a CCW from the chrome compartment to oldWaiver wrapping the window
+  // we just transplanted:
+  //
+  // Compartment 1  |  Compartment 2
+  // ----------------------------------------
+  // CCW1 -----------> oldWaiver --> CCW2 --+
+  // newWaiver                              |
+  // WindowProxy <--------------------------+
+  js::NukeCrossCompartmentWrapperIfExists(cx, js::GetObjectCompartment(newobj),
+                                          oldWaiver);
+
   // Create a waiver in the new compartment. We know there's not one already
   // because we _just_ transplanted, which means that |newobj| was either
   // created from scratch, or was previously cross-compartment wrapper (which
   // should have no waiver). CreateXrayWaiver asserts this.
   JSObject* newWaiver = WrapperFactory::CreateXrayWaiver(cx, newobj);
   if (!newWaiver) {
     return false;
   }