From da05bf53a8bb07059c12205d4302dded0cb2151c Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 3 Jun 2014 11:38:37 -0400 Subject: [PATCH] Bug 989584. Allow sites to set window.opener to any value. r=peterv --- dom/base/nsGlobalWindow.cpp | 120 +++++++++++++++---------- dom/base/nsGlobalWindow.h | 8 +- dom/base/test/test_setting_opener.html | 19 ++++ dom/interfaces/base/nsIDOMWindow.idl | 8 +- dom/webidl/Window.webidl | 2 +- 5 files changed, 103 insertions(+), 54 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 63cd9bdef800..ac409aac0690 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -4339,13 +4339,7 @@ nsGlobalWindow::GetOwnPropertyNames(JSContext* aCx, nsTArray& aNames, nsGlobalWindow::IsChromeWindow(JSContext* aCx, JSObject* aObj) { // For now, have to deal with XPConnect objects here. - nsGlobalWindow* win; - nsresult rv = UNWRAP_OBJECT(Window, aObj, win); - if (NS_FAILED(rv)) { - nsCOMPtr piWin = do_QueryWrapper(aCx, aObj); - win = static_cast(piWin.get()); - } - return win->IsChromeWindow(); + return xpc::WindowOrNull(aObj)->IsChromeWindow(); } nsIDOMOfflineResourceList* @@ -4471,9 +4465,9 @@ nsGlobalWindow::GetControllers(nsIControllers** aResult) } nsIDOMWindow* -nsGlobalWindow::GetOpener(ErrorResult& aError) +nsGlobalWindow::GetOpenerWindow(ErrorResult& aError) { - FORWARD_TO_OUTER_OR_THROW(GetOpener, (aError), aError, nullptr); + FORWARD_TO_OUTER_OR_THROW(GetOpenerWindow, (aError), aError, nullptr); nsCOMPtr opener = do_QueryReferent(mOpener); if (!opener) { @@ -4512,18 +4506,41 @@ nsGlobalWindow::GetOpener(ErrorResult& aError) return nullptr; } +JS::Value +nsGlobalWindow::GetOpener(JSContext* aCx, ErrorResult& aError) +{ + nsCOMPtr opener = GetOpenerWindow(aError); + if (aError.Failed() || !opener) { + return JS::NullValue(); + } + + JS::Rooted val(aCx); + aError = nsContentUtils::WrapNative(aCx, opener, &val); + return val; +} + NS_IMETHODIMP -nsGlobalWindow::GetOpener(nsIDOMWindow** aOpener) +nsGlobalWindow::GetScriptableOpener(JSContext* aCx, + JS::MutableHandle aOpener) { ErrorResult rv; - nsCOMPtr opener = GetOpener(rv); - opener.forget(aOpener); + aOpener.set(GetOpener(aCx, rv)); return rv.ErrorCode(); } +NS_IMETHODIMP +nsGlobalWindow::GetOpener(nsIDOMWindow** aOpener) +{ + ErrorResult rv; + nsCOMPtr opener = GetOpenerWindow(rv); + opener.forget(aOpener); + return rv.ErrorCode(); +} + void -nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener, ErrorResult& aError) +nsGlobalWindow::SetOpener(JSContext* aCx, JS::Handle aOpener, + ErrorResult& aError) { // Check if we were called from a privileged chrome script. If not, and if // aOpener is not null, just define aOpener on our inner window's JS object, @@ -4531,35 +4548,15 @@ nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener, ErrorResult& aError) // Xray expando object, but don't set it on the outer window, so that it'll // get reset on navigation. This is just like replaceable properties, but // we're not quite readonly. - if (aOpener && !nsContentUtils::IsCallerChrome()) { - // JS_WrapObject will outerize, so we don't care if aOpener is an inner. - nsCOMPtr glob = do_QueryInterface(aOpener); - if (!glob) { - aError.Throw(NS_ERROR_UNEXPECTED); - return; - } - - AutoJSContext cx; - JSAutoRequest ar(cx); - // Note we explicitly do NOT enter any particular compartment here; we want - // the caller compartment in cases when we have a caller, so that we define - // expandos on Xrays as needed. - - JS::Rooted otherObj(cx, glob->GetGlobalJSObject()); - if (!otherObj) { - aError.Throw(NS_ERROR_UNEXPECTED); - return; - } - - JS::Rooted thisObj(cx, GetWrapperPreserveColor()); + if (!aOpener.isNull() && !nsContentUtils::IsCallerChrome()) { + JS::Rooted thisObj(aCx, GetWrapperPreserveColor()); if (!thisObj) { aError.Throw(NS_ERROR_UNEXPECTED); return; } - if (!JS_WrapObject(cx, &otherObj) || - !JS_WrapObject(cx, &thisObj) || - !JS_DefineProperty(cx, thisObj, "opener", otherObj, JSPROP_ENUMERATE, + if (!JS_WrapObject(aCx, &thisObj) || + !JS_DefineProperty(aCx, thisObj, "opener", aOpener, JSPROP_ENUMERATE, JS_PropertyStub, JS_StrictPropertyStub)) { aError.Throw(NS_ERROR_FAILURE); } @@ -4567,16 +4564,47 @@ nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener, ErrorResult& aError) return; } - SetOpenerWindow(aOpener, false); + if (!aOpener.isObjectOrNull()) { + // Chrome code trying to set some random value as opener + aError.Throw(NS_ERROR_INVALID_ARG); + return; + } + + nsGlobalWindow* win = nullptr; + if (aOpener.isObject()) { + JSObject* unwrapped = js::CheckedUnwrap(&aOpener.toObject(), + /* stopAtOuter = */ false); + if (!unwrapped) { + aError.Throw(NS_ERROR_DOM_SECURITY_ERR); + return; + } + + win = xpc::WindowOrNull(unwrapped); + if (!win) { + // Wasn't a window + aError.Throw(NS_ERROR_INVALID_ARG); + return; + } + } + + SetOpenerWindow(win, false); +} + +NS_IMETHODIMP +nsGlobalWindow::SetScriptableOpener(JSContext* aCx, + JS::Handle aOpener) +{ + ErrorResult rv; + SetOpener(aCx, aOpener, rv); + + return rv.ErrorCode(); } NS_IMETHODIMP nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener) { - ErrorResult rv; - SetOpener(aOpener, rv); - - return rv.ErrorCode(); + SetOpenerWindow(aOpener, false); + return NS_OK; } void @@ -13699,13 +13727,7 @@ bool nsGlobalWindow::IsModalContentWindow(JSContext* aCx, JSObject* aGlobal) { // For now, have to deal with XPConnect objects here. - nsGlobalWindow* win; - nsresult rv = UNWRAP_OBJECT(Window, aGlobal, win); - if (NS_FAILED(rv)) { - nsCOMPtr piWin = do_QueryWrapper(aCx, aGlobal); - win = static_cast(piWin.get()); - } - return win->IsModalContentWindow(); + return xpc::WindowOrNull(aGlobal)->IsModalContentWindow(); } NS_IMETHODIMP diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index e042d15030f5..c9c76fec401c 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -836,8 +836,12 @@ public: aError = GetScriptableTop(getter_AddRefs(top)); return top.forget(); } - nsIDOMWindow* GetOpener(mozilla::ErrorResult& aError); - void SetOpener(nsIDOMWindow* aOpener, mozilla::ErrorResult& aError); +protected: + nsIDOMWindow* GetOpenerWindow(mozilla::ErrorResult& aError); +public: + JS::Value GetOpener(JSContext* aCx, mozilla::ErrorResult& aError); + void SetOpener(JSContext* aCx, JS::Handle aOpener, + mozilla::ErrorResult& aError); using nsIDOMWindow::GetParent; already_AddRefed GetParent(mozilla::ErrorResult& aError); mozilla::dom::Element* GetFrameElement(mozilla::ErrorResult& aError); diff --git a/dom/base/test/test_setting_opener.html b/dom/base/test/test_setting_opener.html index 247f0595e317..babdf85705b2 100644 --- a/dom/base/test/test_setting_opener.html +++ b/dom/base/test/test_setting_opener.html @@ -59,6 +59,25 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=868996 is(evalsb("win.opener", sb2), window, "Navigating a window should have reset the opener in sb2"); + win.opener = 5; + evalsb("win.opener = 5", sb1); + evalsb("win.opener = 5", sb2); + is(win.opener, 5, "Should be able to set an opener to a primitive"); + is(evalsb("win.opener", sb1), 5, + "Should be able to set the opener to a primitive in a sandbox one"); + is(evalsb("win.opener", sb2), 5, + "Should be able to set the opener to a primitive in a sandbox two"); + win.location = "data:text/html,