git://git.webkit.org / WebKit.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 90283c6)
Fix xssauditor bypass with unterminated closing tag by making the HTMLSourceTracker
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 18:59:21 +0000 (18:59 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 18:59:21 +0000 (18:59 +0000)
and the HTMLParser interact more closely with each other.  HTMLParser should be
setting the end range for the token itself to account for buffering that the
HTMLSourceTracker can't know about, but there are a lot of paths that would need
updating. First step is to cover this one path.
https://bugs.webkit.org/show_bug.cgi?id=68281

Patch by Tom Sepez <tsepez@chromium.org> on 2011-09-19
Reviewed by Adam Barth.

Source/WebCore:

Test: http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html

* html/parser/HTMLSourceTracker.cpp:
(WebCore::HTMLSourceTracker::end):
* html/parser/HTMLTokenizer.cpp:
(WebCore::HTMLTokenizer::nextToken):

LayoutTests:

* http/tests/security/xssAuditor/resources/echo-intertag.pl:
* http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt: Added.
* http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95451 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog patch | blob | history
LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl patch | blob | history
LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/html/parser/HTMLSourceTracker.cpp patch | blob | history
Source/WebCore/html/parser/HTMLTokenizer.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6330eb4..269f074 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2011-09-19  Tom Sepez  <tsepez@chromium.org>
+
+        Fix xssauditor bypass with unterminated closing tag by making the HTMLSourceTracker
+        and the HTMLParser interact more closely with each other.  HTMLParser should be
+        setting the end range for the token itself to account for buffering that the
+        HTMLSourceTracker can't know about, but there are a lot of paths that would need
+        updating. First step is to cover this one path.
+        https://bugs.webkit.org/show_bug.cgi?id=68281
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/xssAuditor/resources/echo-intertag.pl:
+        * http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt: Added.
+        * http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html: Added.
+
 2011-09-19  Dmitry Lomov  <dslomov@google.com>
 
         [Chromium] Rebaseline expectations and file WK68372. 
diff --git a/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl b/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl
index 284900e..9561bd6 100755 (executable)
--- a/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl
+++ b/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl
@@ -32,6 +32,9 @@ if ($cgi->param('relay-target-ids-for-event')) {
 }
 print "<body>\n";
 print $cgi->param('q');
+if ($cgi->param('clutter')) {
+    print $cgi->param('clutter');
+}
 if ($cgi->param('notifyDone')) {
     print "<script>\n";
     print "if (window.layoutTestController)\n";
diff --git a/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt b/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt
new file mode 100644 (file)
index 0000000..513e2f8
--- /dev/null
+++ b/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag-expected.txt
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 1: Refused to execute a JavaScript script. Source code of script found within request.
+
+
diff --git a/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html b/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html
new file mode 100644 (file)
index 0000000..cda4cb7
--- /dev/null
+++ b/LayoutTests/http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+  layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?clutter=%20<i><b>&q=<script>alert(String.fromCharCode(0x58,0x53,0x53))</script">
+</iframe>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 33e3ef7..a782e38 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2011-09-19  Tom Sepez  <tsepez@chromium.org>
+
+        Fix xssauditor bypass with unterminated closing tag by making the HTMLSourceTracker
+        and the HTMLParser interact more closely with each other.  HTMLParser should be
+        setting the end range for the token itself to account for buffering that the
+        HTMLSourceTracker can't know about, but there are a lot of paths that would need
+        updating. First step is to cover this one path.
+        https://bugs.webkit.org/show_bug.cgi?id=68281
+
+        Reviewed by Adam Barth.
+
+        Test: http/tests/security/xssAuditor/script-tag-with-invalid-closing-tag.html
+
+        * html/parser/HTMLSourceTracker.cpp:
+        (WebCore::HTMLSourceTracker::end):
+        * html/parser/HTMLTokenizer.cpp:
+        (WebCore::HTMLTokenizer::nextToken):
+
 2011-09-19  Peter Rybin  <peter.rybin@gmail.com>
 
         TextPosition refactoring: Merge ZeroBasedNumber and OneBasedNumber classes
diff --git a/Source/WebCore/html/parser/HTMLSourceTracker.cpp b/Source/WebCore/html/parser/HTMLSourceTracker.cpp
index b46469b..c53a959 100644 (file)
--- a/Source/WebCore/html/parser/HTMLSourceTracker.cpp
+++ b/Source/WebCore/html/parser/HTMLSourceTracker.cpp
@@ -43,8 +43,12 @@ void HTMLSourceTracker::start(const HTMLInputStream& input, HTMLToken& token)
 void HTMLSourceTracker::end(const HTMLInputStream& input, HTMLToken& token)
 {
     m_cachedSourceForToken = String();
-    // FIXME: This work should really be done by the HTMLTokenizer.
-    token.end(input.current().numberOfCharactersConsumed());
+
+    // FIXME: This work should really be done by the HTMLTokenizer in all cases,
+    // instead of the few cases where it explicitly steps in to correct values
+    // known to be wrong in face of its internal buffering.
+    if (!token.endIndex())
+        token.end(input.current().numberOfCharactersConsumed());
 }
 
 String HTMLSourceTracker::sourceForToken(const HTMLToken& token)
diff --git a/Source/WebCore/html/parser/HTMLTokenizer.cpp b/Source/WebCore/html/parser/HTMLTokenizer.cpp
index 7a269be..118b837 100644 (file)
--- a/Source/WebCore/html/parser/HTMLTokenizer.cpp
+++ b/Source/WebCore/html/parser/HTMLTokenizer.cpp
@@ -297,8 +297,12 @@ bool HTMLTokenizer::nextToken(SegmentedString& source, HTMLToken& token)
     END_STATE()
 
     HTML_BEGIN_STATE(ScriptDataState) {
-        if (cc == '<')
+        if (cc == '<') {
+            // Token might end here. If not, we'll come through here again
+            // and update the end location again.
+            m_token->end(source.numberOfCharactersConsumed());
             HTML_ADVANCE_TO(ScriptDataLessThanSignState);
+        }
         else if (cc == InputStreamPreprocessor::endOfFileMarker)
             return emitEndOfFile(source);
         else {
Mirror of the WebKit open source project on svn.webkit.org.