Title: [97087] trunk
Revision
97087
Author
commit-qu...@webkit.org
Date
2011-10-10 14:28:53 -0700 (Mon, 10 Oct 2011)

Log Message

ScriptController::executeIfJavaScriptURL gets confused by synchronous frame loads
https://bugs.webkit.org/show_bug.cgi?id=69777

Patch by Sergey Glazunov <serg.glazu...@gmail.com> on 2011-10-10
Reviewed by Adam Barth.

Source/WebCore:

Test: http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html

* bindings/ScriptControllerBase.cpp:
(WebCore::ScriptController::executeIfJavaScriptURL):
* loader/DocumentWriter.cpp:
(WebCore::DocumentWriter::replaceDocument):
(WebCore::DocumentWriter::begin):
* loader/DocumentWriter.h:

Source/WebKit/chromium:

* src/WebFrameImpl.cpp:
(WebKit::WebFrameImpl::loadJavaScriptURL):

LayoutTests:

* http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url-expected.txt: Added.
* http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (97086 => 97087)


--- trunk/LayoutTests/ChangeLog	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/LayoutTests/ChangeLog	2011-10-10 21:28:53 UTC (rev 97087)
@@ -1,3 +1,13 @@
+2011-10-10  Sergey Glazunov  <serg.glazu...@gmail.com>
+
+        ScriptController::executeIfJavaScriptURL gets confused by synchronous frame loads
+        https://bugs.webkit.org/show_bug.cgi?id=69777
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url-expected.txt: Added.
+        * http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html: Added.
+
 2011-10-10  Simon Fraser  <simon.fra...@apple.com>
 
         WebKitTestRunner needs layoutTestController.setWindowIsKey

Added: trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url-expected.txt (0 => 97087)


--- trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url-expected.txt	2011-10-10 21:28:53 UTC (rev 97087)
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 1: Unsafe _javascript_ attempt to access frame with URL http://localhost:8080/security/resources/innocent-victim.html from frame with URL about:blank. Domains, protocols and ports must match.
+
+This test passes if there's no alert dialog.  

Added: trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html (0 => 97087)


--- trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html	2011-10-10 21:28:53 UTC (rev 97087)
@@ -0,0 +1,42 @@
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+	layoutTestController.dumpAsText();
+	layoutTestController.waitUntilDone();
+	layoutTestController.setCanOpenWindows();
+	layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+
+window._onload_ = function()
+{
+	victim = document.body.appendChild(document.createElement("iframe"));
+	wnd = victim.contentWindow.open();
+	victim.src = ""
+	victim._onload_ = function() {
+		victim._onload_ = null;
+
+		wnd.eval("(" + function() {
+			location = "_javascript_:(" + function() {
+				a = document.createElement("a");
+				a.href = ""
+				e = document.createEvent("MouseEvent");
+				e.initMouseEvent("click");
+				a.dispatchEvent(e);
+
+				return "<script>(" + function() {
+					opener.location = "_javascript_:alert(document.body.innerHTML)";
+
+					if (window.layoutTestController)
+						setTimeout("layoutTestController.notifyDone()", 0);
+				} + ")()<\/script>";
+			} + ")()";
+		} + ")()");
+	}
+}
+</script>
+</head>
+<body>
+This test passes if there's no alert dialog.
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (97086 => 97087)


--- trunk/Source/WebCore/ChangeLog	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebCore/ChangeLog	2011-10-10 21:28:53 UTC (rev 97087)
@@ -1,3 +1,19 @@
+2011-10-10  Sergey Glazunov  <serg.glazu...@gmail.com>
+
+        ScriptController::executeIfJavaScriptURL gets confused by synchronous frame loads
+        https://bugs.webkit.org/show_bug.cgi?id=69777
+
+        Reviewed by Adam Barth.
+
+        Test: http/tests/security/xss-DENIED-synchronous-frame-load-in-_javascript_-url.html
+
+        * bindings/ScriptControllerBase.cpp:
+        (WebCore::ScriptController::executeIfJavaScriptURL):
+        * loader/DocumentWriter.cpp:
+        (WebCore::DocumentWriter::replaceDocument):
+        (WebCore::DocumentWriter::begin):
+        * loader/DocumentWriter.h:
+
 2011-10-10  Nayan Kumar K  <naya...@motorola.com>
 
         Add missing ifdef for _javascript__DEBUGGER feature.

Modified: trunk/Source/WebCore/bindings/ScriptControllerBase.cpp (97086 => 97087)


--- trunk/Source/WebCore/bindings/ScriptControllerBase.cpp	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebCore/bindings/ScriptControllerBase.cpp	2011-10-10 21:28:53 UTC (rev 97087)
@@ -93,6 +93,7 @@
     // We need to hold onto the Frame here because executing script can
     // destroy the frame.
     RefPtr<Frame> protector(m_frame);
+    RefPtr<Document> ownerDocument(m_frame->document());
 
     const int _javascript_SchemeLength = sizeof("_javascript_:") - 1;
 
@@ -125,7 +126,7 @@
         // DocumentWriter::replaceDocument can cause the DocumentLoader to get deref'ed and possible destroyed,
         // so protect it with a RefPtr.
         if (RefPtr<DocumentLoader> loader = m_frame->document()->loader())
-            loader->writer()->replaceDocument(scriptResult);
+            loader->writer()->replaceDocument(scriptResult, ownerDocument.get());
     }
     return true;
 }

Modified: trunk/Source/WebCore/loader/DocumentWriter.cpp (97086 => 97087)


--- trunk/Source/WebCore/loader/DocumentWriter.cpp	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebCore/loader/DocumentWriter.cpp	2011-10-10 21:28:53 UTC (rev 97087)
@@ -64,10 +64,10 @@
 // This is only called by ScriptController::executeIfJavaScriptURL
 // and always contains the result of evaluating a _javascript_: url.
 // This is the <iframe src="" case.
-void DocumentWriter::replaceDocument(const String& source)
+void DocumentWriter::replaceDocument(const String& source, Document* ownerDocument)
 {
     m_frame->loader()->stopAllLoaders();
-    begin(m_frame->document()->url(), true, InheritSecurityOrigin);
+    begin(m_frame->document()->url(), true, ownerDocument);
 
     if (!source.isNull()) {
         if (!m_hasReceivedSomeData) {
@@ -106,10 +106,8 @@
     return DOMImplementation::createDocument(m_mimeType, m_frame, url, m_frame->inViewSourceMode());
 }
 
-void DocumentWriter::begin(const KURL& urlReference, bool dispatch, SecurityOriginSource originSource)
+void DocumentWriter::begin(const KURL& urlReference, bool dispatch, Document* ownerDocument)
 {
-    RefPtr<Document> oldDocument = m_frame->document();
-
     // We grab a local copy of the URL because it's easy for callers to supply
     // a URL that will be deallocated during the execution of this function.
     // For example, see <https://bugs.webkit.org/show_bug.cgi?id=66360>.
@@ -137,9 +135,9 @@
 
     if (m_decoder)
         document->setDecoder(m_decoder.get());
-    if (originSource == InheritSecurityOrigin) {
-        document->setCookieURL(oldDocument->cookieURL());
-        document->setSecurityOrigin(oldDocument->securityOrigin());
+    if (ownerDocument) {
+        document->setCookieURL(ownerDocument->cookieURL());
+        document->setSecurityOrigin(ownerDocument->securityOrigin());
     }
 
     m_frame->domWindow()->setURL(document->url());

Modified: trunk/Source/WebCore/loader/DocumentWriter.h (97086 => 97087)


--- trunk/Source/WebCore/loader/DocumentWriter.h	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebCore/loader/DocumentWriter.h	2011-10-10 21:28:53 UTC (rev 97087)
@@ -47,12 +47,10 @@
 
     // This is only called by ScriptController::executeIfJavaScriptURL
     // and always contains the result of evaluating a _javascript_: url.
-    void replaceDocument(const String&);
+    void replaceDocument(const String&, Document* ownerDocument);
 
-    enum SecurityOriginSource { CreateNewSecurityOrigin, InheritSecurityOrigin };
-
     void begin();
-    void begin(const KURL&, bool dispatchWindowObjectAvailable = true, SecurityOriginSource = CreateNewSecurityOrigin);
+    void begin(const KURL&, bool dispatchWindowObjectAvailable = true, Document* ownerDocument = 0);
     void addData(const char* bytes, size_t length);
     void end();
     void endIfNotLoadingMainResource();

Modified: trunk/Source/WebKit/chromium/ChangeLog (97086 => 97087)


--- trunk/Source/WebKit/chromium/ChangeLog	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebKit/chromium/ChangeLog	2011-10-10 21:28:53 UTC (rev 97087)
@@ -1,3 +1,13 @@
+2011-10-10  Sergey Glazunov  <serg.glazu...@gmail.com>
+
+        ScriptController::executeIfJavaScriptURL gets confused by synchronous frame loads
+        https://bugs.webkit.org/show_bug.cgi?id=69777
+
+        Reviewed by Adam Barth.
+
+        * src/WebFrameImpl.cpp:
+        (WebKit::WebFrameImpl::loadJavaScriptURL):
+
 2011-10-10  Dominic Mazzoni  <dmazz...@google.com>
 
         [Chromium] Get rid of WebAccessibilityCache.

Modified: trunk/Source/WebKit/chromium/src/WebFrameImpl.cpp (97086 => 97087)


--- trunk/Source/WebKit/chromium/src/WebFrameImpl.cpp	2011-10-10 21:21:41 UTC (rev 97086)
+++ trunk/Source/WebKit/chromium/src/WebFrameImpl.cpp	2011-10-10 21:28:53 UTC (rev 97087)
@@ -2255,6 +2255,8 @@
     if (!m_frame->document() || !m_frame->page())
         return;
 
+    RefPtr<Document> ownerDocument(m_frame->document());
+
     // Protect privileged pages against bookmarklets and other _javascript_ manipulations.
     if (SchemeRegistry::shouldTreatURLSchemeAsNotAllowingJavascriptURLs(m_frame->document()->url().protocol()))
         return;
@@ -2267,7 +2269,7 @@
         return;
 
     if (!m_frame->navigationScheduler()->locationChangePending())
-        m_frame->document()->loader()->writer()->replaceDocument(scriptResult);
+        m_frame->document()->loader()->writer()->replaceDocument(scriptResult, ownerDocument.get());
 }
 
 } // namespace WebKit
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to