Reviewed by Darin.
Fix for <rdar://problem/
5592988>
- Enforce tighter restrictions on what frames in other domains
can be navigated.
Tests: http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html
http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html
* bindings/js/kjs_window.cpp:
(KJS::Window::put):
(KJS::Location::put):
(KJS::LocationProtoFuncReplace::callAsFunction):
(KJS::LocationProtoFuncAssign::callAsFunction):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::createWindow):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::shouldAllowNavigation): Move and update logic from canTarget().
* loader/FrameLoader.h:
* page/FrameTree.cpp:
(WebCore::FrameTree::isDescendantOf): Make this O(1) in the case when both frames are not
in the same page.
LayoutTests:
Reviewed by Darin.
Tests for <rdar://problem/
5592988>
- Update and add tests for new tighter restrictions on what frames in other domains
can be navigated.
* http/tests/security/cross-frame-access-location-expected.txt:
* http/tests/security/frameNavigation: Added.
* http/tests/security/frameNavigation/resources: Added.
* http/tests/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html: Added.
* http/tests/security/frameNavigation/resources/iframe-with-inner-frame-on-foreign-domain.html: Added.
* http/tests/security/frameNavigation/resources/navigation-changed-iframe.html: Added.
* http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-expected.txt: Added.
* http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html: Added.
* http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change-expected.txt: Added.
* http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28056
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-11-26 Sam Weinig <sam@webkit.org>
+
+ Reviewed by Darin.
+
+ Tests for <rdar://problem/5592988>
+
+ - Update and add tests for new tighter restrictions on what frames in other domains
+ can be navigated.
+
+ * http/tests/security/cross-frame-access-location-expected.txt:
+ * http/tests/security/frameNavigation: Added.
+ * http/tests/security/frameNavigation/resources: Added.
+ * http/tests/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html: Added.
+ * http/tests/security/frameNavigation/resources/iframe-with-inner-frame-on-foreign-domain.html: Added.
+ * http/tests/security/frameNavigation/resources/navigation-changed-iframe.html: Added.
+ * http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-expected.txt: Added.
+ * http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html: Added.
+ * http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change-expected.txt: Added.
+ * http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html: Added.
+
2007-11-26 Dan Bernstein <mitz@apple.com>
Reviewed by Dave Hyatt.
CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-get-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-location.html. Domains, protocols and ports must match.
-CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-get-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-location.html. Domains, protocols and ports must match.
-
----- tests for getting/setting window.location and its properties -----
--- /dev/null
+<html>
+<head>
+ <script>
+ function loaded()
+ {
+ document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+ // Allow the user to click the button during manuel runs.
+ if (window.layoutTestController)
+ performTest();
+ }
+
+ function performTest()
+ {
+ parent.location = "http://localhost:8000/security/frameNavigation/resources/navigation-changed-iframe.html";
+ }
+ </script>
+</head>
+<body onload="loaded();">
+ <h4>DOMAIN</h4>
+ <button onclick="performTest();">Perform Test</button>
+</body>
+</html>
--- /dev/null
+<html>
+<head>
+ <script>
+ function loaded()
+ {
+ document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+ }
+ </script>
+</head>
+<body onload="loaded();">
+ <h4>DOMAIN</h4>
+ <iframe name="targetFrame" src="http://localhost:8000/security/resources/cross-frame-iframe.html"></iframe>
+</body>
+</html>
--- /dev/null
+<html>
+<head>
+ <script>
+ function fireSentinel()
+ {
+ document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+ </script>
+</head>
+<body onload="fireSentinel();">
+ <h4>DOMAIN</h4>
+ <p>PASSED: Navigation succeeded.</p>
+</body>
+</html>
--- /dev/null
+localhost
+
+PASSED: Navigation succeeded.
--- /dev/null
+<html>
+<head>
+ <style>
+ iframe { width: 400px; height: 200px;}
+ </style>
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ function loaded()
+ {
+ document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+ }
+ </script>
+</head>
+<body onload="loaded();">
+ <p>This tests that documents can navigate the location of any of it's parent-frames regardless of domain.</p>
+ <h4>DOMAIN</h4>
+ <iframe src="http://localhost:8000/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html"></iframe>
+</body>
+</html>
--- /dev/null
+This tests that documents can navigate the location of any of it's sub-frames regardless of domain.
+
+127.0.0.1
+
+ Perform Test
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+localhost
+
+
+
+--------
+Frame: 'targetFrame'
+--------
+localhost
+
+PASSED: Navigation succeeded.
--- /dev/null
+<html>
+<head>
+ <style>
+ iframe { width: 400px; height: 200px;}
+ </style>
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.dumpChildFramesAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ function loaded()
+ {
+ document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+
+ if (window.layoutTestController) {
+ setTimeout(waitForFlag, 1);
+ function waitForFlag() {
+ if (!layoutTestController.globalFlag) {
+ setTimeout(waitForFlag, 1);
+ return;
+ }
+
+ layoutTestController.globalFlag = false;
+ performTest();
+ }
+ }
+ }
+
+ function performTest()
+ {
+ var subFrame = window.frames[0];
+ subFrame.frames[0].location = "http://localhost:8000/security/frameNavigation/resources/navigation-changed-iframe.html";
+ }
+ </script>
+</head>
+<body onload="loaded();">
+ <p>This tests that documents can navigate the location of any of it's sub-frames regardless of domain.</p>
+ <h4>DOMAIN</h4>
+ <iframe src="http://localhost:8000/security/frameNavigation/resources/iframe-with-inner-frame-on-foreign-domain.html"></iframe>
+ <button onclick="performTest()">Perform Test</button>
+</body>
+</html>
+2007-11-26 Sam Weinig <sam@webkit.org>
+
+ Reviewed by Darin.
+
+ Fix for <rdar://problem/5592988>
+ - Enforce tighter restrictions on what frames in other domains
+ can be navigated.
+
+ Tests: http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html
+ http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html
+
+ * bindings/js/kjs_window.cpp:
+ (KJS::Window::put):
+ (KJS::Location::put):
+ (KJS::LocationProtoFuncReplace::callAsFunction):
+ (KJS::LocationProtoFuncAssign::callAsFunction):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::createWindow):
+ (WebCore::FrameLoader::load):
+ (WebCore::FrameLoader::shouldAllowNavigation): Move and update logic from canTarget().
+ * loader/FrameLoader.h:
+ * page/FrameTree.cpp:
+ (WebCore::FrameTree::isDescendantOf): Make this O(1) in the case when both frames are not
+ in the same page.
+
2007-11-26 Steve Falkenburg <sfalken@apple.com>
Build fix.
case Location_: {
Frame* p = Window::retrieveActive(exec)->impl()->frame();
if (p) {
+ if (!p->loader()->shouldAllowNavigation(impl()->frame()))
+ return;
DeprecatedString dstUrl = p->loader()->completeURL(DeprecatedString(value->toString(exec))).url();
if (!dstUrl.startsWith("javascript:", false) || isSafeScript(exec)) {
bool userGesture = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter())->wasRunByUserGesture();
switch (entry->value.intValue) {
case Href: {
Frame* frame = Window::retrieveActive(exec)->impl()->frame();
- if (frame)
- url = frame->loader()->completeURL(str).url();
- else
- url = str;
+ if (!frame)
+ return;
+ if (!frame->loader()->shouldAllowNavigation(m_frame))
+ return;
+ url = frame->loader()->completeURL(str).url();
break;
}
case Hash: {
if (!frame)
return jsUndefined();
- DeprecatedString str = args[0]->toString(exec);
Frame* p = Window::retrieveActive(exec)->impl()->frame();
- if ( p ) {
+ if (p) {
+ if (!p->loader()->shouldAllowNavigation(frame))
+ return jsUndefined();
+ DeprecatedString str = args[0]->toString(exec);
const Window* window = Window::retrieveWindow(frame);
if (!str.startsWith("javascript:", false) || (window && window->isSafeScript(exec))) {
bool userGesture = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter())->wasRunByUserGesture();
if (!frame)
return jsUndefined();
- Window* window = Window::retrieveWindow(frame);
- if (!window->isSafeScript(exec))
- return jsUndefined();
-
Frame *p = Window::retrieveActive(exec)->impl()->frame();
if (p) {
+ if (!p->loader()->shouldAllowNavigation(frame))
+ return jsUndefined();
const Window *window = Window::retrieveWindow(frame);
DeprecatedString dstUrl = p->loader()->completeURL(DeprecatedString(args[0]->toString(exec))).url();
if (!dstUrl.startsWith("javascript:", false) || (window && window->isSafeScript(exec))) {
if (!request.frameName().isEmpty() && request.frameName() != "_blank")
if (Frame* frame = m_frame->tree()->find(request.frameName())) {
+ if (!shouldAllowNavigation(frame))
+ return 0;
if (!request.resourceRequest().url().isEmpty())
frame->loader()->load(request, false, true, 0, 0, HashMap<String, String>());
if (Page* page = frame->page())
referrer = String();
Frame* targetFrame = m_frame->tree()->find(request.frameName());
- if (!canTarget(targetFrame))
+ if (!shouldAllowNavigation(targetFrame))
return;
if (request.resourceRequest().httpMethod() != "POST") {
load(loader.get(), FrameLoadTypeReload, 0);
}
-bool FrameLoader::canTarget(Frame* target) const
+bool FrameLoader::shouldAllowNavigation(Frame* targetFrame) const
{
- // This function prevents this exploit:
+ // This function prevents these exploits:
// <rdar://problem/3715785> multiple frame injection vulnerability reported by Secunia, affects almost all browsers
+ // http://bugs.webkit.org/show_bug.cgi?id=15936 Overly permissive frame navigation allows password theft
// Allow if there is no specific target.
- if (!target)
+ if (!targetFrame)
+ return true;
+ if (m_frame == targetFrame)
return true;
- // Allow navigation within the same page/frameset.
- if (m_frame->page() == target->page())
+ // The navigation change is safe if the active frame is:
+ // - in the same security domain (satisfies same-origin policy)
+ // - the opener frame
+ // - an ancestor or a descendant in frame tree hierarchy
+
+ // Same security domain case.
+ Document* activeDocument = m_frame->document();
+ ASSERT(activeDocument);
+ Document* targetDocument = targetFrame->document();
+ if (!targetDocument)
+ return true;
+ const SecurityOrigin& activeSecurityOrigin = activeDocument->securityOrigin();
+ const SecurityOrigin& targetSecurityOrigin = targetDocument->securityOrigin();
+ if (activeSecurityOrigin.canAccess(targetSecurityOrigin))
return true;
- // Allow if the request is made from a local file.
- ASSERT(m_frame->document());
- String domain = m_frame->document()->domain();
- if (domain.isEmpty())
+ // Opener case.
+ if (m_frame == targetFrame->loader()->opener())
return true;
-
- // Allow if target is an entire window (top level frame of a window).
- Frame* parent = target->tree()->parent();
- if (!parent)
+
+ // Ancestor or descendant case.
+ if (targetFrame->tree()->isDescendantOf(m_frame) || m_frame->tree()->isDescendantOf(targetFrame))
return true;
+
+ if (!targetFrame->settings()->privateBrowsingEnabled()) {
+ // FIXME: this error message should contain more specifics of why the navigation change is not allowed.
+ String message = String::format("Unsafe JavaScript attempt to initiate a navigation change for frame with URL %s from frame with URL %s.\n",
+ targetDocument->URL().utf8().data(), activeDocument->URL().utf8().data());
+
+ if (KJS::Interpreter::shouldPrintExceptions())
+ printf("%s", message.utf8().data());
+
+ // FIXME: should we print to the console of the activeFrame as well?
+ if (Page* page = targetFrame->page())
+ page->chrome()->addMessageToConsole(JSMessageSource, ErrorMessageLevel, message, 1, String());
+ }
- // Allow if the domain of the parent of the targeted frame equals this domain.
- String parentDomain;
- if (Document* parentDocument = parent->document())
- parentDomain = parentDocument->domain();
- return equalIgnoringCase(parentDomain, domain);
+ return false;
}
void FrameLoader::stopLoadingSubframes()
bool committingFirstRealLoad() const { return !m_creatingInitialEmptyDocument && !m_committedFirstRealDocumentLoad; }
void iconLoadDecisionAvailable();
+
+ bool shouldAllowNavigation(Frame* targetFrame) const;
+
private:
PassRefPtr<HistoryItem> createHistoryItem(bool useOriginal);
PassRefPtr<HistoryItem> createHistoryItemTree(Frame* targetFrame, bool clipAtTarget);
void applyUserAgent(ResourceRequest& request);
- bool canTarget(Frame*) const;
-
void scheduleRedirection(ScheduledRedirection*);
void startRedirectionTimer();
void stopRedirectionTimer();
bool FrameTree::isDescendantOf(const Frame* ancestor) const
{
+ if (m_thisFrame->page() != ancestor->page())
+ return false;
+
for (Frame* frame = m_thisFrame; frame; frame = frame->tree()->parent())
if (frame == ancestor)
return true;
git://git.webkit.org / WebKit-https.git / commitdiff