Bug 725490 - Change XFO sameorigin to check all ancestors for same origin. r=smaug
authorJonathan Kingston <jkt@mozilla.com>
Fri, 03 Nov 2017 15:37:10 +0000
changeset 449311 351c75ab74c9a83db5c0662ba271b49479adb1f1
parent 449310 4d1423f75c677447cafb8cd3f9a5fbdff8bfbca8
child 449312 f73719c150e65186c9341edb7afe0fcda8edb897
child 449332 9773358ff522076df6e04f7855ee0fd9133787a1
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs725490
milestone59.0a1
first release with
nightly linux32
351c75ab74c9 / 59.0a1 / 20180102220230 / files
nightly linux64
351c75ab74c9 / 59.0a1 / 20180102220230 / files
nightly mac
351c75ab74c9 / 59.0a1 / 20180102220230 / files
nightly win32
351c75ab74c9 / 59.0a1 / 20180102220230 / files
nightly win64
351c75ab74c9 / 59.0a1 / 20180102220230 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
-