HTMLSourceTracker crashes when network packets break poorly

https://bugs.webkit.org/show_bug.cgi?id=66728 Reviewed by Darin Adler. Source/WebCore: If there is a network packet boundary in the middle of an attribute that begins with the letters "on", then the HTMLSourceTracker will get confused and try to extract too many characters from future input. If the future input is small enough, that will walk off the end of the input and crash. Test: http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html * html/parser/HTMLSourceTracker.cpp: (WebCore::HTMLSourceTracker::sourceForToken): LayoutTests: Test that we don't crash when we get a bad network packet boundary. * http/tests/security/xssAuditor/crash-while-loading-tag-with-pause-expected.txt: Added. * http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html: Added. * http/tests/security/xssAuditor/resources/tag-with-pause.php: Added. Canonical link: https://commits.webkit.org/82540@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@93561 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Aug 22, 2011 · 1938757 · 1938757
1 parent 6dd2b7b
commit 1938757
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 0 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2011-08-22  Adam Barth  <abarth@webkit.org>
+
+        HTMLSourceTracker crashes when network packets break poorly
+        https://bugs.webkit.org/show_bug.cgi?id=66728
+
+        Reviewed by Darin Adler.
+
+        Test that we don't crash when we get a bad network packet boundary.
+
+        * http/tests/security/xssAuditor/crash-while-loading-tag-with-pause-expected.txt: Added.
+        * http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html: Added.
+        * http/tests/security/xssAuditor/resources/tag-with-pause.php: Added.
+
 2011-08-22  Peter Kasting  <pkasting@google.com>
 
         Unreviewed, rolling out r93538.

diff --git a/LayoutTests/http/tests/security/xssAuditor/crash-while-loading-tag-with-pause-expected.txt b/LayoutTests/http/tests/security/xssAuditor/crash-while-loading-tag-with-pause-expected.txt
@@ -0,0 +1,2 @@
+This test passes if it doesn't crash.
+
diff --git a/LayoutTests/http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html b/LayoutTests/http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html
@@ -0,0 +1,8 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+This test passes if it doesn't crash.<br>
+<iframe src="resources/tag-with-pause.php?onclick=alert(1)"></iframe>
diff --git a/LayoutTests/http/tests/security/xssAuditor/resources/tag-with-pause.php b/LayoutTests/http/tests/security/xssAuditor/resources/tag-with-pause.php
@@ -0,0 +1,18 @@
+<?
+header("Content-Type: text/html; charset=utf-8");
+?>
+<!-- <?php
+# Spam a bunch of As to make sure we blow past any buffers.
+print str_repeat("A", 2048);
+?> -->
+<body>
+<?php
+print "<a ona";
+print str_repeat("a", 2000);
+
+flush();
+usleep(200000);
+
+print "click=alert(1) ttt>";
+?>
+Done.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2011-08-22  Adam Barth  <abarth@webkit.org>
+
+        HTMLSourceTracker crashes when network packets break poorly
+        https://bugs.webkit.org/show_bug.cgi?id=66728
+
+        Reviewed by Darin Adler.
+
+        If there is a network packet boundary in the middle of an attribute
+        that begins with the letters "on", then the HTMLSourceTracker will get
+        confused and try to extract too many characters from future input.  If
+        the future input is small enough, that will walk off the end of the
+        input and crash.
+
+        Test: http/tests/security/xssAuditor/crash-while-loading-tag-with-pause.html
+
+        * html/parser/HTMLSourceTracker.cpp:
+        (WebCore::HTMLSourceTracker::sourceForToken):
+
 2011-08-22  Eric Seidel  <eric@webkit.org>
 
         Attempt to clean up bidiNext usage

diff --git a/Source/WebCore/html/parser/HTMLSourceTracker.cpp b/Source/WebCore/html/parser/HTMLSourceTracker.cpp
@@ -60,6 +60,7 @@ String HTMLSourceTracker::sourceForToken(const HTMLToken& token)
     StringBuilder source;
     source.reserveCapacity(length);
     source.append(m_sourceFromPreviousSegments);
+    length -= m_sourceFromPreviousSegments.length();
     for (int i = 0; i < length; ++i) {
         source.append(*m_source);
         m_source.advance();