From 85f5946629156e2ccac68aafa80108414c20deee Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Thu, 18 Jul 2019 15:02:59 +0000 Subject: [PATCH] Bug 1510760, part 5 - Support local-to-remote window proxy transplanting. r=tcampbell,peterv When a BrowsingContext changes from being local to remote, we have to change all window proxies from being local to remote, using transplanting. The actual window proxy becomes a remote window proxy. Cross compartment wrappers (CCWs) to the window proxy also become remote window proxies in their respective compartments, rather than CCWs to a remote proxy in the old compartment of the window proxy, because the window is no longer actually in that compartment. This also avoids having to figure out what Xray behavior for remote window proxies should be. This patch uses the transplanting support I added to GetRemoteOuterWindowProxy() in the previous patch to ensure that the remote proxy map holds the correct value after transplanting finishes. It drops the requirement that both arguments to JS_TransplantObject have the same class, because we need to transplant a window proxy with a remote window proxy. It also deals with this by not adding origobj to the wrapper map unless it is a CCW, to handle transplanting to a remote proxy. The core design here, with the remote window proxies in every compartment, is taken from a patch by peterv. Differential Revision: https://phabricator.services.mozilla.com/D35730 --HG-- extra : moz-landing-system : lando --- docshell/base/BrowsingContext.cpp | 14 ++-- dom/base/nsGlobalWindowOuter.cpp | 45 +++++++++++++ dom/base/nsGlobalWindowOuter.h | 3 + .../browser/browser_windowProxy_transplant.js | 61 +++++++++++++---- js/src/jsapi.cpp | 9 +-- js/src/proxy/CrossCompartmentWrapper.cpp | 7 +- js/src/vm/Compartment.cpp | 3 + js/xpconnect/wrappers/WrapperFactory.cpp | 65 +++++++++++++++++-- 8 files changed, 182 insertions(+), 25 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 227d9dac3275..c6f9b8675a15 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -337,14 +337,20 @@ void BrowsingContext::PrepareForProcessChange() { mIsInProcess = false; - // XXX: We should transplant our WindowProxy into a Cross-Process WindowProxy - // if mWindowProxy is non-nullptr. (bug 1510760) - mWindowProxy = nullptr; - // NOTE: For now, clear our nsDocShell reference, as we're primarily in a // different process now. This may need to change in the future with // Cross-Process BFCache. mDocShell = nullptr; + + if (!mWindowProxy) { + return; + } + + // We have to go through mWindowProxy rather than calling GetDOMWindow() on + // mDocShell because the mDocshell reference gets cleared immediately after + // the window is closed. + nsGlobalWindowOuter::PrepareForProcessChange(mWindowProxy); + MOZ_ASSERT(!mWindowProxy); } void BrowsingContext::CacheChildren(bool aFromIPC) { diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index 1b17a11c2db7..5601073c23ec 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -2284,6 +2284,51 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument, return NS_OK; } +/* static */ +void nsGlobalWindowOuter::PrepareForProcessChange(JSObject* aProxy) { + MOZ_ASSERT(js::IsWindowProxy(aProxy)); + + RefPtr outerWindow = + nsOuterWindowProxy::GetOuterWindow(aProxy); + if (!outerWindow) { + return; + } + + AutoJSAPI jsapi; + jsapi.Init(); + JSContext* cx = jsapi.cx(); + + JS::Rooted localProxy(cx, aProxy); + JSAutoRealm ar(cx, localProxy); + + // Clear out existing references from the browsing context and outer window to + // the proxy, and from the proxy to the outer window. These references will + // become invalid once the proxy is transplanted. Clearing the window proxy + // from the browsing context is also necessary to indicate that it is for an + // out of process window. + outerWindow->ClearWrapper(localProxy); + RefPtr bc = outerWindow->GetBrowsingContext(); + MOZ_ASSERT(bc); + MOZ_ASSERT(bc->GetWindowProxy() == localProxy); + bc->ClearWindowProxy(); + js::SetProxyReservedSlot(localProxy, OUTER_WINDOW_SLOT, + js::PrivateValue(nullptr)); + js::SetProxyReservedSlot(localProxy, HOLDER_WEAKMAP_SLOT, + JS::UndefinedValue()); + + // Create a new remote outer window proxy, and transplant to it. + JS::Rooted remoteProxy(cx); + + if (!mozilla::dom::GetRemoteOuterWindowProxy(cx, bc, localProxy, + &remoteProxy)) { + MOZ_CRASH("PrepareForProcessChange GetRemoteOuterWindowProxy"); + } + + if (!xpc::TransplantObject(cx, localProxy, remoteProxy)) { + MOZ_CRASH("PrepareForProcessChange TransplantObject"); + } +} + void nsGlobalWindowOuter::PreloadLocalStorage() { if (!Storage::StoragePrefIsEnabled()) { return; diff --git a/dom/base/nsGlobalWindowOuter.h b/dom/base/nsGlobalWindowOuter.h index 6e1557460dfa..296a09c32428 100644 --- a/dom/base/nsGlobalWindowOuter.h +++ b/dom/base/nsGlobalWindowOuter.h @@ -315,6 +315,9 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget, virtual nsresult SetNewDocument(Document* aDocument, nsISupports* aState, bool aForceReuseInnerWindow) override; + // Outer windows only. + static void PrepareForProcessChange(JSObject* aProxy); + // Outer windows only. void DispatchDOMWindowCreated(); diff --git a/dom/tests/browser/browser_windowProxy_transplant.js b/dom/tests/browser/browser_windowProxy_transplant.js index 2103b58ac24c..df9195362a72 100644 --- a/dom/tests/browser/browser_windowProxy_transplant.js +++ b/dom/tests/browser/browser_windowProxy_transplant.js @@ -36,7 +36,7 @@ add_task(async function() { BrowserTestUtils.loadURI(browser, URL1); await BrowserTestUtils.browserLoaded(browser, false, URL1); - info("Browser has loaded initial URI."); + info("Chrome script has loaded initial URI."); await ContentTask.spawn( browser, { URL1, URL2, URL3 }, @@ -44,7 +44,7 @@ add_task(async function() { let iframe = content.document.createElement("iframe"); content.document.body.appendChild(iframe); - info("iframe created"); + info("Chrome script created iframe"); // Here and below, we have to store references to things in the // iframes on the content window, because all chrome references @@ -89,6 +89,7 @@ add_task(async function() { await waitLoad(); content.win1 = iframe.contentWindow; + let chromeWin1 = iframe.contentWindow; content.bc1 = iframe.browsingContext; is( @@ -96,15 +97,16 @@ add_task(async function() { content.bc1, "same to same-origin BrowsingContext match" ); - ok( - content.win0 == content.win1, - "same to same-origin WindowProxy match" - ); + is(content.win0, content.win1, "same to same-origin WindowProxy match"); ok( !Cu.isDeadWrapper(content.win1), "win1 shouldn't be a dead wrapper before navigation" ); + ok( + !Cu.isDeadWrapper(chromeWin1), + "chromeWin1 shouldn't be a dead wrapper before navigation" + ); askLoad(URL2); await waitLoad(); @@ -112,13 +114,49 @@ add_task(async function() { content.win2 = iframe.contentWindow; content.bc2 = iframe.browsingContext; + // When chrome accesses a remote window proxy in content, the result + // should be a remote outer window proxy in the chrome compartment, not an + // Xray wrapper around the content remote window proxy. The former will + // throw a security error, because @@toPrimitive can't be called cross + // process, while the latter will result in an opaque wrapper, because + // XPConnect doesn't know what to do when trying to create an Xray wrapper + // around a remote outer window proxy. See bug 1556845. + Assert.throws( + () => { + dump("content.win1 " + content.win1 + "\n"); + }, + /SecurityError: Permission denied to access property Symbol.toPrimitive on cross-origin object/, + "Should get a remote outer window proxy when accessing old window proxy" + ); + Assert.throws( + () => { + dump("content.win2 " + content.win2 + "\n"); + }, + /SecurityError: Permission denied to access property Symbol.toPrimitive on cross-origin object/, + "Should get a remote outer window proxy when accessing new window proxy" + ); + + // If we fail to transplant existing non-remote outer window proxies, then + // after we navigate the iframe existing chrome references to the window will + // become dead wrappers. Also check content.win1 for thoroughness, though + // we don't nuke content-content references. + ok( + !Cu.isDeadWrapper(content.win1), + "win1 shouldn't be a dead wrapper after navigation" + ); + ok( + !Cu.isDeadWrapper(chromeWin1), + "chromeWin1 shouldn't be a dead wrapper after navigation" + ); + is( content.bc1, content.bc2, "same to cross-origin navigation BrowsingContext match" ); - todo( - content.win1 == content.win2, + is( + content.win1, + content.win2, "same to cross-origin navigation WindowProxy match" ); @@ -138,8 +176,9 @@ add_task(async function() { content.bc3, "cross to cross-origin navigation BrowsingContext match" ); - ok( - content.win2 == content.win3, + is( + content.win2, + content.win3, "cross to cross-origin navigation WindowProxy match" ); @@ -156,7 +195,7 @@ add_task(async function() { ); todo( content.win3 == content.win4, - "cross to same-origin navigation WindowProxty match" + "cross to same-origin navigation WindowProxy match" ); } ); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 711dc6e26988..4e7249e4b54b 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -667,7 +667,6 @@ JS_PUBLIC_API JSObject* JS_TransplantObject(JSContext* cx, HandleObject origobj, MOZ_ASSERT(origobj != target); MOZ_ASSERT(!origobj->is()); MOZ_ASSERT(!target->is()); - MOZ_ASSERT(origobj->getClass() == target->getClass()); ReleaseAssertObjectHasNoWrappers(cx, target); JS::AssertCellIsNotGray(origobj); JS::AssertCellIsNotGray(target); @@ -726,9 +725,11 @@ JS_PUBLIC_API JSObject* JS_TransplantObject(JSContext* cx, HandleObject origobj, } MOZ_ASSERT(Wrapper::wrappedObject(newIdentityWrapper) == newIdentity); JSObject::swap(cx, origobj, newIdentityWrapper); - if (!origobj->compartment()->putWrapper( - cx, CrossCompartmentKey(newIdentity), origv)) { - MOZ_CRASH(); + if (origobj->compartment()->lookupWrapper(newIdentity)) { + MOZ_ASSERT(origobj->is()); + if (!origobj->compartment()->putWrapper(cx, CrossCompartmentKey(newIdentity), origv)) { + MOZ_CRASH(); + } } } diff --git a/js/src/proxy/CrossCompartmentWrapper.cpp b/js/src/proxy/CrossCompartmentWrapper.cpp index f45b7bee86f2..c022507c42be 100644 --- a/js/src/proxy/CrossCompartmentWrapper.cpp +++ b/js/src/proxy/CrossCompartmentWrapper.cpp @@ -593,13 +593,18 @@ void js::RemapWrapper(JSContext* cx, JSObject* wobjArg, JSObject::swap(cx, wobj, tobj); } + if (!wobj->is()) { + MOZ_ASSERT(js::IsProxy(wobj) && js::GetProxyHandler(wobj)->family() == + js::GetDOMRemoteProxyHandlerFamily()); + return; + } + // Before swapping, this wrapper came out of rewrap(), which enforces the // invariant that the wrapper in the map points directly to the key. MOZ_ASSERT(Wrapper::wrappedObject(wobj) == newTarget); // Update the entry in the compartment's wrapper map to point to the old // wrapper, which has now been updated (via reuse or swap). - MOZ_ASSERT(wobj->is()); if (!wcompartment->putWrapper(cx, CrossCompartmentKey(newTarget), ObjectValue(*wobj))) { oomUnsafe.crash("js::RemapWrapper"); diff --git a/js/src/vm/Compartment.cpp b/js/src/vm/Compartment.cpp index f365fc1bcfcc..f9702b42a4db 100644 --- a/js/src/vm/Compartment.cpp +++ b/js/src/vm/Compartment.cpp @@ -68,6 +68,9 @@ bool Compartment::putWrapper(JSContext* cx, const CrossCompartmentKey& wrapped, const js::Value& wrapper) { MOZ_ASSERT(wrapped.is() == wrapper.isString()); MOZ_ASSERT_IF(!wrapped.is(), wrapper.isObject()); + MOZ_ASSERT(!wrapper.isObject() || !js::IsProxy(&wrapper.toObject()) || + js::GetProxyHandler(&wrapper.toObject())->family() != + js::GetDOMRemoteProxyHandlerFamily()); if (!crossCompartmentWrappers.put(wrapped, wrapper)) { ReportOutOfMemory(cx); diff --git a/js/xpconnect/wrappers/WrapperFactory.cpp b/js/xpconnect/wrappers/WrapperFactory.cpp index 9ff0c177b4cb..eb08c8b32b9f 100644 --- a/js/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/xpconnect/wrappers/WrapperFactory.cpp @@ -152,6 +152,58 @@ inline bool ShouldWaiveXray(JSContext* cx, JSObject* originalObj) { return sameOrigin; } +// Special handling is needed when wrapping local and remote window proxies. +// This function returns true if it found a window proxy and dealt with it. +static bool MaybeWrapWindowProxy(JSContext* cx, HandleObject origObj, + HandleObject obj, MutableHandleObject retObj) { + bool isWindowProxy = js::IsWindowProxy(obj); + + if (!isWindowProxy && + !dom::IsRemoteObjectProxy(obj, dom::prototypes::id::Window)) { + return false; + } + + dom::BrowsingContext* bc = nullptr; + if (isWindowProxy) { + nsGlobalWindowInner* win = + WindowOrNull(js::UncheckedUnwrap(obj, /* stopAtWindowProxy = */ false)); + if (win && win->GetOuterWindow()) { + bc = win->GetOuterWindow()->GetBrowsingContext(); + } + if (!bc) { + retObj.set(obj); + return true; + } + } else { + bc = dom::GetBrowsingContext(obj); + MOZ_ASSERT(bc); + } + + if (bc->IsInProcess()) { + // If the bc is in process, we should have a local window proxy for it + // already. + MOZ_ASSERT(bc->GetWindowProxy()); + + if (isWindowProxy) { + retObj.set(obj); + } else { + // XXX Turning remote window proxies into local window proxies will be + // implemented as part of bug 1559489, so don't allow it for now. To + // implement bug 1559489, we can return bc->GetWindowProxy() for the + // remote proxy case. + retObj.set(JS_NewDeadWrapper(cx)); + } + } else { + // If bc is not in process, then use a remote window proxy, whether or not + // obj is one already. + if (!dom::GetRemoteOuterWindowProxy(cx, bc, origObj, retObj)) { + MOZ_CRASH("GetRemoteOuterWindowProxy failed"); + } + } + + return true; +} + void WrapperFactory::PrepareForWrapping(JSContext* cx, HandleObject scope, HandleObject origObj, HandleObject objArg, @@ -165,11 +217,14 @@ void WrapperFactory::PrepareForWrapping(JSContext* cx, HandleObject scope, RootedObject obj(cx, objArg); retObj.set(nullptr); - // 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); + // There are a few cases related to window proxies that are handled first to + // allow us to assert against wrappers below. + if (MaybeWrapWindowProxy(cx, origObj, obj, retObj)) { + if (waive) { + // We don't put remote window proxies in a waiving wrapper. + MOZ_ASSERT(js::IsWindowProxy(obj)); + retObj.set(WaiveXray(cx, retObj)); + } return; }