Bug 1092388 - Clamp the entry and incumbent global to the current global if their principals don't match. r=bz, a=lsblakk
authorBobby Holley <bobbyholley@gmail.com>
Thu, 13 Nov 2014 11:13:36 -0800
changeset 233905 0671ae35933f2b8916ce702f33355bef4df9536e
parent 233904 f0e0ea2e07343976fa9c97631904129d0566982c
child 233906 5531b615a4850fba67342aa6768f6bc99460cbba
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, lsblakk
bugs1092388
milestone35.0a2
Bug 1092388 - Clamp the entry and incumbent global to the current global if their principals don't match. r=bz, a=lsblakk
dom/base/ScriptSettings.cpp
toolkit/components/viewsource/test/test_428653.xul
--- a/dom/base/ScriptSettings.cpp
+++ b/dom/base/ScriptSettings.cpp
@@ -117,20 +117,54 @@ ScriptSettingsStackEntry::ScriptSettings
 ScriptSettingsStackEntry::~ScriptSettingsStackEntry()
 {
   // We must have an actual JS global for the entire time this is on the stack.
   MOZ_ASSERT_IF(mGlobalObject, mGlobalObject->GetGlobalJSObject());
 
   ScriptSettingsStack::Pop(this);
 }
 
+// If the entry or incumbent global ends up being something that the subject
+// principal doesn't subsume, we don't want to use it. This never happens on
+// the web, but can happen with asymmetric privilege relationships (i.e.
+// nsExpandedPrincipal and System Principal).
+//
+// The most correct thing to use instead would be the topmost global on the
+// callstack whose principal is subsumed by the subject principal. But that's
+// hard to compute, so we just substitute the global of the current
+// compartment. In practice, this is fine.
+//
+// Note that in particular things like:
+//
+// |SpecialPowers.wrap(crossOriginWindow).eval(open())|
+//
+// trigger this case. Although both the entry global and the current global
+// have normal principals, the use of Gecko-specific System-Principaled JS
+// puts the code from two different origins on the callstack at once, which
+// doesn't happen normally on the web.
+static nsIGlobalObject*
+ClampToSubject(nsIGlobalObject* aGlobalOrNull)
+{
+  if (!aGlobalOrNull || !NS_IsMainThread()) {
+    return aGlobalOrNull;
+  }
+
+  nsIPrincipal* globalPrin = aGlobalOrNull->PrincipalOrNull();
+  NS_ENSURE_TRUE(globalPrin, GetCurrentGlobal());
+  if (!nsContentUtils::SubjectPrincipal()->SubsumesConsideringDomain(globalPrin)) {
+    return GetCurrentGlobal();
+  }
+
+  return aGlobalOrNull;
+}
+
 nsIGlobalObject*
 GetEntryGlobal()
 {
-  return ScriptSettingsStack::EntryGlobal();
+  return ClampToSubject(ScriptSettingsStack::EntryGlobal());
 }
 
 nsIDocument*
 GetEntryDocument()
 {
   nsCOMPtr<nsPIDOMWindow> entryWin = do_QueryInterface(GetEntryGlobal());
   return entryWin ? entryWin->GetExtantDoc() : nullptr;
 }
@@ -149,22 +183,22 @@ GetIncumbentGlobal()
     return nullptr;
   }
 
   // See what the JS engine has to say. If we've got a scripted caller
   // override in place, the JS engine will lie to us and pretend that
   // there's nothing on the JS stack, which will cause us to check the
   // incumbent script stack below.
   if (JSObject *global = JS::GetScriptedCallerGlobal(cx)) {
-    return xpc::NativeGlobal(global);
+    return ClampToSubject(xpc::NativeGlobal(global));
   }
 
   // Ok, nothing from the JS engine. Let's use whatever's on the
   // explicit stack.
-  return ScriptSettingsStack::IncumbentGlobal();
+  return ClampToSubject(ScriptSettingsStack::IncumbentGlobal());
 }
 
 nsIGlobalObject*
 GetCurrentGlobal()
 {
   JSContext *cx = nsContentUtils::GetCurrentJSContextForThread();
   if (!cx) {
     return nullptr;
--- a/toolkit/components/viewsource/test/test_428653.xul
+++ b/toolkit/components/viewsource/test/test_428653.xul
@@ -12,18 +12,19 @@
   <html:body/>
 
   <browser id="content" type="content-primary" name="content" src="about:blank" flex="1"
            disablehistory="true" context="viewSourceContextMenu"/>
 
   <script type="application/javascript">
   <![CDATA[
   /*
-  Test that we can't call the content browser's document.open() either via
-  wrappedJSObject (SafeJSObjectWrapper) or not.
+  Test that we can't call the content browser's document.open() over Xrays.
+  See the security checks in nsHTMLDocument::Open, which make sure that the
+  entry global's principal matches that of the document.
   */
   SimpleTest.waitForExplicitFinish();
   
   addLoadEvent(function testDocumentOpen() {
     var browser = document.getElementById("content");
     ok(browser, "got browser");
     var doc = browser.contentDocument;
     ok(doc, "got content document");
@@ -32,22 +33,16 @@
     try {
       doc.open("text/html", "replace");
       opened = true;
     } catch (e) {
       is(e.name, "SecurityError", "Unxpected exception")
     }
     is(opened, false, "Shouldn't have opened document");
 
-    try {
-      doc.wrappedJSObject.open("text/html", "replace");
-      opened = true;
-    } catch (e) {
-      is(e.name, "SecurityError",
-         "Unxpected exception via wrappedJSObject")
-    }
-    is(opened, false, "Shouldn't have opened document via wrappedJSObject");
+    doc.wrappedJSObject.open("text/html", "replace");
+    ok(true, "Should be able to open document via Xray Waiver");
 
     SimpleTest.finish();
   });
   ]]>
   </script>
 </window>