Bug 725490 - Change XFO sameorigin to check all ancestors for same origin. r?smaug draft
authorJonathan Kingston <jkt@mozilla.com>
Fri, 03 Nov 2017 15:37:10 +0000
changeset 715159 2153147cf348eac13200e6dcef7ae2647bbb1f4e
parent 714633 f5a1cb52c12e8fbcf2e3b5a675fe2a84d43507a7
child 744714 7060488746552ea96cd248fa69fd462d125431c0
push id94073
push userbmo:jkt@mozilla.com
push dateTue, 02 Jan 2018 12:15:42 +0000
reviewerssmaug
bugs725490
milestone59.0a1
Bug 725490 - Change XFO sameorigin to check all ancestors for same origin. r?smaug MozReview-Commit-ID: 5fPxGpcdVms
dom/security/FramingChecker.cpp
testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
--- a/dom/security/FramingChecker.cpp
+++ b/dom/security/FramingChecker.cpp
@@ -73,16 +73,21 @@ FramingChecker::CheckOneFrameOptionsPoli
   nsCOMPtr<nsIDocument> topDoc;
   nsresult rv;
   nsCOMPtr<nsIScriptSecurityManager> ssm =
     do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
   if (!ssm) {
     MOZ_CRASH();
   }
 
+  // If the X-Frame-Options value is SAMEORIGIN, then the top frame in the
+  // parent chain must be from the same origin as this document.
+  bool checkSameOrigin = aPolicy.LowerCaseEqualsLiteral("sameorigin");
+  nsCOMPtr<nsIURI> topUri;
+
   // Traverse up the parent chain and stop when we see a docshell whose
   // parent has a system principal, or a docshell corresponding to
   // <iframe mozbrowser>.
   while (NS_SUCCEEDED(
            curDocShellItem->GetParent(getter_AddRefs(parentDocShellItem))) &&
          parentDocShellItem) {
     nsCOMPtr<nsIDocShell> curDocShell = do_QueryInterface(curDocShellItem);
     if (curDocShell && curDocShell->GetIsMozBrowser()) {
@@ -93,16 +98,27 @@ FramingChecker::CheckOneFrameOptionsPoli
     topDoc = parentDocShellItem->GetDocument();
     if (topDoc) {
       if (NS_SUCCEEDED(
             ssm->IsSystemPrincipal(topDoc->NodePrincipal(), &system)) &&
           system) {
         // Found a system-principled doc: last docshell was top.
         break;
       }
+
+      if (checkSameOrigin) {
+        topDoc->NodePrincipal()->GetURI(getter_AddRefs(topUri));
+        rv = ssm->CheckSameOriginURI(uri, topUri, true);
+
+        // one of the ancestors is not same origin as this document
+        if (NS_FAILED(rv)) {
+          ReportXFOViolation(curDocShellItem, uri, eSAMEORIGIN);
+          return false;
+        }
+      }
     } else {
       return false;
     }
     curDocShellItem = parentDocShellItem;
   }
 
   // If this document has the top non-SystemPrincipal docshell it is not being
   // framed or it is being framed by a chrome document, which we allow.
@@ -114,29 +130,18 @@ FramingChecker::CheckOneFrameOptionsPoli
   // not met (current docshell is not the top docshell), prohibit the
   // load.
   if (aPolicy.LowerCaseEqualsLiteral("deny")) {
     ReportXFOViolation(curDocShellItem, uri, eDENY);
     return false;
   }
 
   topDoc = curDocShellItem->GetDocument();
-  nsCOMPtr<nsIURI> topUri;
   topDoc->NodePrincipal()->GetURI(getter_AddRefs(topUri));
 
-  // If the X-Frame-Options value is SAMEORIGIN, then the top frame in the
-  // parent chain must be from the same origin as this document.
-  if (aPolicy.LowerCaseEqualsLiteral("sameorigin")) {
-    rv = ssm->CheckSameOriginURI(uri, topUri, true);
-    if (NS_FAILED(rv)) {
-      ReportXFOViolation(curDocShellItem, uri, eSAMEORIGIN);
-      return false; /* wasn't same-origin */
-    }
-  }
-
   // If the X-Frame-Options value is "allow-from [uri]", then the top
   // frame in the parent chain must be from that origin
   if (isAllowFrom) {
     if (aPolicy.Length() == allowFromLen ||
         (aPolicy[allowFromLen] != ' ' &&
          aPolicy[allowFromLen] != '\t')) {
       ReportXFOViolation(curDocShellItem, uri, eALLOWFROM);
       return false;
--- a/testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
+++ b/testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
@@ -1,8 +1,5 @@
 [sameorigin.sub.html]
   type: testharness
   [`XFO: SAMEORIGIN` blocks cross-origin framing.]
     expected: FAIL
 
-  [`XFO: SAMEORIGIN` blocks same-origin nested in cross-origin framing.]
-    expected: FAIL
-