Bug 725490 - Change XFO sameorigin to check all ancestors for same origin. r?smaug
MozReview-Commit-ID: 5fPxGpcdVms
--- 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
-