Bug 989584. Allow sites to set window.opener to any value. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 03 Jun 2014 11:38:37 -0400
changeset 205594 026bd94e5bd06146e64acd9d36c40da0fd35cea1
parent 205593 27cd25212b6232d2833dbf046c1bd2514397ff57
child 205595 15aa216cebf99dc17be83984f29e7c6db700fe21
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs989584
milestone32.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 989584. Allow sites to set window.opener to any value. r=peterv
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/base/test/test_setting_opener.html
dom/interfaces/base/nsIDOMWindow.idl
dom/webidl/Window.webidl
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -4334,23 +4334,17 @@ nsGlobalWindow::GetOwnPropertyNames(JSCo
     nameSpaceManager->EnumerateGlobalNames(EnumerateGlobalName, &closure);
   }
 }
 
 /* static */ bool
 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<nsPIDOMWindow> piWin = do_QueryWrapper(aCx, aObj);
-    win = static_cast<nsGlobalWindow*>(piWin.get());
-  }
-  return win->IsChromeWindow();
+  return xpc::WindowOrNull(aObj)->IsChromeWindow();
 }
 
 nsIDOMOfflineResourceList*
 nsGlobalWindow::GetApplicationCache(ErrorResult& aError)
 {
   FORWARD_TO_INNER_OR_THROW(GetApplicationCache, (aError), aError, nullptr);
 
   if (!mApplicationCache) {
@@ -4466,19 +4460,19 @@ nsGlobalWindow::GetControllers(nsIContro
   ErrorResult rv;
   nsCOMPtr<nsIControllers> controllers = GetControllers(rv);
   controllers.forget(aResult);
 
   return rv.ErrorCode();
 }
 
 nsIDOMWindow*
-nsGlobalWindow::GetOpener(ErrorResult& aError)
-{
-  FORWARD_TO_OUTER_OR_THROW(GetOpener, (aError), aError, nullptr);
+nsGlobalWindow::GetOpenerWindow(ErrorResult& aError)
+{
+  FORWARD_TO_OUTER_OR_THROW(GetOpenerWindow, (aError), aError, nullptr);
 
   nsCOMPtr<nsPIDOMWindow> opener = do_QueryReferent(mOpener);
   if (!opener) {
     return nullptr;
   }
 
   // First, check if we were called from a privileged chrome script
   if (nsContentUtils::IsCallerChrome()) {
@@ -4507,81 +4501,115 @@ nsGlobalWindow::GetOpener(ErrorResult& a
         return opener;
       }
     }
   }
 
   return nullptr;
 }
 
+JS::Value
+nsGlobalWindow::GetOpener(JSContext* aCx, ErrorResult& aError)
+{
+  nsCOMPtr<nsIDOMWindow> opener = GetOpenerWindow(aError);
+  if (aError.Failed() || !opener) {
+    return JS::NullValue();
+  }
+
+  JS::Rooted<JS::Value> val(aCx);
+  aError = nsContentUtils::WrapNative(aCx, opener, &val);
+  return val;
+}
+
+NS_IMETHODIMP
+nsGlobalWindow::GetScriptableOpener(JSContext* aCx,
+                                    JS::MutableHandle<JS::Value> aOpener)
+{
+  ErrorResult rv;
+  aOpener.set(GetOpener(aCx, rv));
+
+  return rv.ErrorCode();
+}
+
 NS_IMETHODIMP
 nsGlobalWindow::GetOpener(nsIDOMWindow** aOpener)
 {
   ErrorResult rv;
-  nsCOMPtr<nsIDOMWindow> opener = GetOpener(rv);
+  nsCOMPtr<nsIDOMWindow> opener = GetOpenerWindow(rv);
   opener.forget(aOpener);
-
-  return rv.ErrorCode();
-}
-
-void
-nsGlobalWindow::SetOpener(nsIDOMWindow* aOpener, ErrorResult& aError)
+  return rv.ErrorCode();
+}
+
+void
+nsGlobalWindow::SetOpener(JSContext* aCx, JS::Handle<JS::Value> 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,
   // wrapped into the current compartment so that for Xrays we define on the
   // 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<nsIGlobalObject> 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<JSObject*> otherObj(cx, glob->GetGlobalJSObject());
-    if (!otherObj) {
-      aError.Throw(NS_ERROR_UNEXPECTED);
-      return;
-    }
-
-    JS::Rooted<JSObject*> thisObj(cx, GetWrapperPreserveColor());
+  if (!aOpener.isNull() && !nsContentUtils::IsCallerChrome()) {
+    JS::Rooted<JSObject*> 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);
     }
 
     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<JS::Value> 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
 nsGlobalWindow::GetStatus(nsAString& aStatus, ErrorResult& aError)
 {
   FORWARD_TO_OUTER_OR_THROW(GetStatus, (aStatus, aError), aError, );
 
   aStatus = mStatus;
@@ -13694,23 +13722,17 @@ nsGlobalModalWindow::SetReturnValue(nsIV
   return NS_OK;
 }
 
 /* static */
 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<nsPIDOMWindow> piWin = do_QueryWrapper(aCx, aGlobal);
-    win = static_cast<nsGlobalWindow*>(piWin.get());
-  }
-  return win->IsModalContentWindow();
+  return xpc::WindowOrNull(aGlobal)->IsModalContentWindow();
 }
 
 NS_IMETHODIMP
 nsGlobalWindow::GetConsole(JSContext* aCx,
                            JS::MutableHandle<JS::Value> aConsole)
 {
   ErrorResult rv;
   nsRefPtr<Console> console = GetConsole(rv);
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -831,18 +831,22 @@ public:
   already_AddRefed<nsIDOMWindow> GetFrames(mozilla::ErrorResult& aError);
   uint32_t Length();
   already_AddRefed<nsIDOMWindow> GetTop(mozilla::ErrorResult& aError)
   {
     nsCOMPtr<nsIDOMWindow> top;
     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<JS::Value> aOpener,
+                 mozilla::ErrorResult& aError);
   using nsIDOMWindow::GetParent;
   already_AddRefed<nsIDOMWindow> GetParent(mozilla::ErrorResult& aError);
   mozilla::dom::Element* GetFrameElement(mozilla::ErrorResult& aError);
   already_AddRefed<nsIDOMWindow> Open(const nsAString& aUrl,
                                       const nsAString& aName,
                                       const nsAString& aOptions,
                                       mozilla::ErrorResult& aError);
   mozilla::dom::Navigator* GetNavigator(mozilla::ErrorResult& aError);
--- a/dom/base/test/test_setting_opener.html
+++ b/dom/base/test/test_setting_opener.html
@@ -54,16 +54,35 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   function continueOpenerTest(win) {
     is(win.opener, window, "Navigating a window should have reset the opener we stashed on it temporarily");
     is(evalsb("win.opener", sb1), window,
        "Navigating a window should have reset the opener in sb1");
     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,<script>opener.setTimeout(opener.continueOpenerTest2, 0, this);</" + "script>";
+  }
+
+  function continueOpenerTest2(win) {
+    is(win.opener, window,
+       "Navigating a window again should have reset the opener we stashed on it temporarily");
+    is(evalsb("win.opener", sb1), window,
+       "Navigating a window again should have reset the opener in sb1");
+    is(evalsb("win.opener", sb2), window,
+       "Navigating a window again should have reset the opener in sb2");
+
     win.opener = null;
     is(win.opener, null, "Should be able to set the opener to null");
     is(evalsb("win.opener", sb1), null,
        "Setting the opener to null should be visible in sb1");
     is(evalsb("win.opener", sb2), null,
        "Setting the opener to null should be visible in sb2");
 
     win.location = "data:text/html,Loaded";
--- a/dom/interfaces/base/nsIDOMWindow.idl
+++ b/dom/interfaces/base/nsIDOMWindow.idl
@@ -19,17 +19,17 @@ interface nsIVariant;
  * The nsIDOMWindow interface is the primary interface for a DOM
  * window object. It represents a single window object that may
  * contain child windows if the document in the window contains a
  * HTML frameset document or if the document contains iframe elements.
  *
  * @see <http://www.whatwg.org/html/#window>
  */
 
-[scriptable, uuid(fbefa573-0ba2-4d15-befb-d60277643a0b)]
+[scriptable, uuid(d4316591-d16e-405c-8093-b441cbef3230)]
 interface nsIDOMWindow : nsISupports
 {
   // the current browsing context
   readonly attribute nsIDOMWindow                       window;
 
   /* [replaceable] self */
   readonly attribute nsIDOMWindow                       self;
 
@@ -146,17 +146,21 @@ interface nsIDOMWindow : nsISupports
 
   %{C++
   inline nsresult GetParent(nsIDOMWindow **aWindow)
   {
     return GetRealParent(aWindow);
   }
   %}
 
-           attribute nsIDOMWindow                       opener;
+  [implicit_jscontext, binaryname(ScriptableOpener)]
+           attribute jsval                              opener;
+
+  [noscript, binaryname(Opener)]
+           attribute nsIDOMWindow                       openerWindow;
 
   /**
    * |frameElement| gets this window's <iframe> or <frame> element, if it has
    * one.
    *
    * When script reads this property (or when C++ calls
    * ScriptableFrameElement), we return |null| if the window is inside an
    * <iframe mozbrowser>.  In contrast, when C++ calls GetFrameElement, we
--- a/dom/webidl/Window.webidl
+++ b/dom/webidl/Window.webidl
@@ -49,17 +49,17 @@ typedef any Transferable;
   [Throws, CrossOriginCallable] void focus();
   [Throws, CrossOriginCallable] void blur();
 
   // other browsing contexts
   [Replaceable, Throws, CrossOriginReadable] readonly attribute WindowProxy frames;
   [Replaceable, CrossOriginReadable] readonly attribute unsigned long length;
   //[Unforgeable, Throws, CrossOriginReadable] readonly attribute WindowProxy top;
   [Unforgeable, Throws, CrossOriginReadable] readonly attribute WindowProxy? top;
-  [Throws, CrossOriginReadable] attribute WindowProxy? opener;
+  [Throws, CrossOriginReadable] attribute any opener;
   //[Throws] readonly attribute WindowProxy parent;
   [Replaceable, Throws, CrossOriginReadable] readonly attribute WindowProxy? parent;
   [Throws] readonly attribute Element? frameElement;
   //[Throws] WindowProxy open(optional DOMString url = "about:blank", optional DOMString target = "_blank", [TreatNullAs=EmptyString] optional DOMString features = "", optional boolean replace = false);
   [Throws] WindowProxy? open(optional DOMString url = "", optional DOMString target = "", [TreatNullAs=EmptyString] optional DOMString features = "");
   // We think the indexed getter is a bug in the spec, it actually needs to live
   // on the WindowProxy
   //getter WindowProxy (unsigned long index);