Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(550)

Unified Diff: Source/core/dom/Document.cpp

Issue 303793003: Make mixed content checking and CSP aware of RemoteFrames (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 9a539716bb24931040285c6e207c7d8fd6825959..bb6d038094898228053251b2d3260590adebe955 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -311,7 +311,7 @@ static bool acceptsEditingFocus(const Element& element)
return element.document().frame() && element.rootEditableElement();
}
-static bool canAccessAncestor(const SecurityOrigin& activeSecurityOrigin, LocalFrame* targetFrame)
+static bool canAccessAncestor(const SecurityOrigin& activeSecurityOrigin, Frame* targetFrame)
{
// targetFrame can be 0 when we're trying to navigate a top-level frame
// that has a 0 opener.
@@ -319,8 +319,13 @@ static bool canAccessAncestor(const SecurityOrigin& activeSecurityOrigin, LocalF
return false;
const bool isLocalActiveOrigin = activeSecurityOrigin.isLocal();
- for (LocalFrame* ancestorFrame = targetFrame; ancestorFrame; ancestorFrame = ancestorFrame->tree().parent()) {
- Document* ancestorDocument = ancestorFrame->document();
+ for (Frame* ancestorFrame = targetFrame; ancestorFrame; ancestorFrame = ancestorFrame->tree().parent()) {
+ // FIXME: SecurityOrigins need to be refactored to work with out-of-process iframes.
+ // For now we prevent navigation between cross-process frames.
+ if (!ancestorFrame->isLocalFrame())
+ return false;
+
+ Document* ancestorDocument = toLocalFrame(ancestorFrame)->document();
// FIXME: Should be an ASSERT? Frames should alway have documents.
if (!ancestorDocument)
return true;
@@ -2923,7 +2928,7 @@ void Document::disableEval(const String& errorMessage)
frame()->script().disableEval(errorMessage);
}
-bool Document::canNavigate(LocalFrame* targetFrame)
+bool Document::canNavigate(Frame* targetFrame)
{
if (!m_frame)
return false;
@@ -2934,7 +2939,7 @@ bool Document::canNavigate(LocalFrame* targetFrame)
if (!targetFrame)
return true;
- // LocalFrame-busting is generally allowed, but blocked for sandboxed frames lacking the 'allow-top-navigation' flag.
+ // Frame-busting is generally allowed, but blocked for sandboxed frames lacking the 'allow-top-navigation' flag.
if (!isSandboxed(SandboxTopNavigation) && targetFrame == m_frame->tree().top())
Mike West 2014/05/29 09:47:41 Sandboxing flags will be moving up to Frame as wel
kenrb 2014/05/29 13:38:41 Those come off of the HTMLFrameOwnerElement. dchen
return true;
@@ -2946,7 +2951,7 @@ bool Document::canNavigate(LocalFrame* targetFrame)
if (isSandboxed(SandboxTopNavigation) && targetFrame == m_frame->tree().top())
reason = "The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.";
- printNavigationErrorMessage(*targetFrame, url(), reason);
+ printNavigationErrorMessage(*toLocalFrameTemporary(targetFrame), url(), reason);
return false;
}
@@ -2978,23 +2983,27 @@ bool Document::canNavigate(LocalFrame* targetFrame)
if (targetFrame == m_frame->loader().opener())
return true;
- if (canAccessAncestor(origin, targetFrame->loader().opener()))
+ // If targetFrame is a RemoteFrame then it is a different origin.
Mike West 2014/05/29 09:47:41 True, but that doesn't mean that canAccessAncestor
kenrb 2014/05/29 13:38:41 Good point. I have added a TODO to reflect that.
+ if (targetFrame->isLocalFrame() && canAccessAncestor(origin, toLocalFrame(targetFrame)->loader().opener()))
return true;
}
- printNavigationErrorMessage(*targetFrame, url(), "The frame attempting navigation is neither same-origin with the target, nor is it the target's parent or opener.");
+ printNavigationErrorMessage(*toLocalFrameTemporary(targetFrame), url(), "The frame attempting navigation is neither same-origin with the target, nor is it the target's parent or opener.");
return false;
}
LocalFrame* Document::findUnsafeParentScrollPropagationBoundary()
{
LocalFrame* currentFrame = m_frame;
- LocalFrame* ancestorFrame = currentFrame->tree().parent();
+ Frame* ancestorFrame = currentFrame->tree().parent();
while (ancestorFrame) {
- if (!ancestorFrame->document()->securityOrigin()->canAccess(securityOrigin()))
+ // RemoteFrames always have different origins.
Mike West 2014/05/29 09:47:41 Same comment regarding canAccess as above; differe
kenrb 2014/05/29 13:38:41 Comment changed.
+ if (!ancestorFrame->isLocalFrame())
+ return currentFrame;
+ if (!toLocalFrame(ancestorFrame)->document()->securityOrigin()->canAccess(securityOrigin()))
return currentFrame;
- currentFrame = ancestorFrame;
+ currentFrame = toLocalFrame(ancestorFrame);
ancestorFrame = ancestorFrame->tree().parent();
}
return 0;
@@ -3234,6 +3243,7 @@ String Document::outgoingReferrer()
Document* referrerDocument = this;
if (LocalFrame* frame = m_frame) {
while (frame->document()->isSrcdocDocument()) {
+ // Srcdoc documents must be local within the containing frame.
Mike West 2014/05/29 09:47:41 Perhaps you could add `ASSERT(frame->isLocal())` t
kenrb 2014/05/29 13:38:41 That will implicitly happen in the last stage of t
frame = frame->tree().parent();
// Srcdoc documents cannot be top-level documents, by definition,
// because they need to be contained in iframes with the srcdoc.
@@ -4837,7 +4847,7 @@ void Document::initSecurityContext(const DocumentInit& initializer)
void Document::initContentSecurityPolicy(const ContentSecurityPolicyResponseHeaders& headers)
{
- if (m_frame && m_frame->tree().parent() && (shouldInheritSecurityOriginFromOwner(m_url) || isPluginDocument()))
+ if (m_frame && m_frame->tree().parent() && m_frame->tree().parent()->isLocalFrame() && (shouldInheritSecurityOriginFromOwner(m_url) || isPluginDocument()))
Mike West 2014/05/29 09:47:41 Why does my parent need to be a local frame for th
kenrb 2014/05/29 13:38:41 I probably should have mentioned this when I put t
contentSecurityPolicy()->copyStateFrom(m_frame->tree().parent()->document()->contentSecurityPolicy());
contentSecurityPolicy()->didReceiveHeaders(headers);
}

Powered by Google App Engine
This is Rietveld 408576698