Framesniffing defense is too aggressive.
https://bugs.webkit.org/show_bug.cgi?id=83721
Reviewed by James Robinson.
Source/WebCore:
The RenderLayer code currently propagates scroll position to parent frames
without any cross-origin checks. This gives it a quick origin boundary check
that is set by FrameLoader only when performing a fragment navigation. This
allows us to safely relax the restriction on not scrolling at load time in
FrameLoader since the safe thing will happen later on at scroll time.
Test: http/tests/navigation/anchor-frames-same-origin.html
* dom/Document.cpp:
(WebCore::Document::findUnsafeParentScrollPropagationBoundary):
* dom/Document.h:
(Document):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::finishedParsing):
(WebCore::FrameLoader::loadInSameDocument):
(WebCore::FrameLoader::scrollToFragmentWithParentBoundary):
* loader/FrameLoader.h:
(FrameLoader):
* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::FrameView::reset):
* page/FrameView.h:
(WebCore::FrameView::safeToPropagateScrollToParent):
(WebCore::FrameView::setSafeToPropagateScrollToParent):
(FrameView):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):
LayoutTests:
* http/tests/inspector/resource-parameters-expected.txt:
* http/tests/navigation/anchor-frames-cross-origin-expected.txt:
* http/tests/navigation/anchor-frames-cross-origin.html:
* http/tests/navigation/anchor-frames-same-origin-expected.txt: Added.
* http/tests/navigation/anchor-frames-same-origin.html: Added.
* http/tests/navigation/resources/frame-with-anchor-cross-origin.html:
* http/tests/navigation/resources/frame-with-anchor-same-origin.html: Added.
* http/tests/navigation/resources/grandchild-with-anchor.html: Added.
* http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt:
* http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event-expected.txt:
* http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event-null-char-expected.txt:
* http/tests/security/xssAuditor/anchor-url-dom-write-location-javascript-URL-expected.txt:
* http/tests/security/xssAuditor/anchor-url-dom-write-location2-expected.txt:
* http/tests/security/xssAuditor/dom-write-location-inline-event-expected.txt:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@114406 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 1de8374..b1dc02c 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -662,7 +662,7 @@
// Check if the scrollbars are really needed for the content.
// If not, remove them, relayout, and repaint.
m_frame->view()->restoreScrollbar();
- scrollToFragmentIfAllowed(m_frame->document()->url());
+ scrollToFragmentWithParentBoundary(m_frame->document()->url());
}
void FrameLoader::loadDone()
@@ -1029,7 +1029,7 @@
// We need to scroll to the fragment whether or not a hash change occurred, since
// the user might have scrolled since the previous navigation.
- scrollToFragmentIfAllowed(url);
+ scrollToFragmentWithParentBoundary(url);
m_isComplete = false;
checkCompleted();
@@ -2618,20 +2618,22 @@
&& !m_frame->document()->isFrameSet();
}
-void FrameLoader::scrollToFragmentIfAllowed(const KURL& url)
+void FrameLoader::scrollToFragmentWithParentBoundary(const KURL& url)
{
FrameView* view = m_frame->view();
if (!view)
return;
// Leaking scroll position to a cross-origin ancestor would permit the so-called "framesniffing" attack.
- if (url.hasFragmentIdentifier() && !m_frame->document()->canBeAccessedByEveryAncestorFrame()) {
- DEFINE_STATIC_LOCAL(String, consoleMessage, ("Fragment navigation not allowed with cross-origin frames."));
- m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage);
- return;
- }
+ RefPtr<Frame> boundaryFrame(url.hasFragmentIdentifier() ? m_frame->document()->findUnsafeParentScrollPropagationBoundary() : 0);
+
+ if (boundaryFrame)
+ boundaryFrame->view()->setSafeToPropagateScrollToParent(false);
view->scrollToFragment(url);
+
+ if (boundaryFrame)
+ boundaryFrame->view()->setSafeToPropagateScrollToParent(true);
}
void FrameLoader::callContinueLoadAfterNavigationPolicy(void* argument,
