Bug 1092388 - Clamp the entry and incumbent global to the current global if their principals don't match. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Thu, 13 Nov 2014 11:13:36 -0800
changeset 239964 366564330ff308d499e39c84a27ac32ef23ace4e
parent 239963 7de86d3b2a08457438ea6ed25bea2b8908af3808
child 239965 56185630acb59387545aa2965ab6d77d29d21b7b
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1092388
milestone36.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 1092388 - Clamp the entry and incumbent global to the current global if their principals don't match. r=bz
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()
 {
   nsIGlobalObject* global = GetEntryGlobal();
   nsCOMPtr<nsPIDOMWindow> entryWin = do_QueryInterface(global);
 
@@ -158,22 +192,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>