Diff
Modified: trunk/LayoutTests/ChangeLog (166201 => 166202)
--- trunk/LayoutTests/ChangeLog 2014-03-24 23:01:56 UTC (rev 166201)
+++ trunk/LayoutTests/ChangeLog 2014-03-24 23:11:43 UTC (rev 166202)
@@ -1,3 +1,17 @@
+2014-03-24 Daniel Bates <[email protected]>
+
+ XSS Auditor doesn't block <script> injected before an existing <script>
+ https://bugs.webkit.org/show_bug.cgi?id=130475
+
+ Merged from Blink (patch by Tom Sepez):
+ https://src.chromium.org/viewvc/blink?view=rev&revision=169697
+
+ * http/tests/security/xssAuditor/resources/echo-intertag.pl:
+ * http/tests/security/xssAuditor/script-tag-_expression_-follows-expected.txt: Added.
+ * http/tests/security/xssAuditor/script-tag-_expression_-follows.html: Added.
+ * http/tests/security/xssAuditor/script-tag-near-start-expected.txt: Added.
+ * http/tests/security/xssAuditor/script-tag-near-start.html: Added.
+
2014-03-24 Brent Fulgham <[email protected]>
Activate WebVTT Tests Once Merging is Complete
Modified: trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl (166201 => 166202)
--- trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl 2014-03-24 23:01:56 UTC (rev 166201)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl 2014-03-24 23:11:43 UTC (rev 166202)
@@ -92,7 +92,10 @@
if ($cgi->param('replaceState')) {
print "<script>history.replaceState({}, '', '#must-not-appear');</script>\n";
}
-print $cgi->param('q');
+print $cgi->param('q'); # XSS reflected here.
+if ($cgi->param('script-_expression_-follows')) {
+ print "\n <script>42;</script>\n";
+}
if ($cgi->param('clutter')) {
print $cgi->param('clutter');
}
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows-expected.txt (0 => 166202)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows-expected.txt 2014-03-24 23:11:43 UTC (rev 166202)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 5: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?script-_expression_-follows=1&q=%3Cscript%3Ealert('XSS')' because its source code was found within the request. The auditor was enabled as the server sent neither an 'X-XSS-Protection' nor 'Content-Security-Policy' header.
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows.html (0 => 166202)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-_expression_-follows.html 2014-03-24 23:11:43 UTC (rev 166202)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=""
+</iframe>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start-expected.txt (0 => 166202)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start-expected.txt 2014-03-24 23:11:43 UTC (rev 166202)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 5: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?script-_expression_-follows=1&q=%3Cscript%3E%22%3Cscript%3E%22-alert(/XSS/)' because its source code was found within the request. The auditor was enabled as the server sent neither an 'X-XSS-Protection' nor 'Content-Security-Policy' header.
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start.html (0 => 166202)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-near-start.html 2014-03-24 23:11:43 UTC (rev 166202)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=""
+</iframe>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (166201 => 166202)
--- trunk/Source/WebCore/ChangeLog 2014-03-24 23:01:56 UTC (rev 166201)
+++ trunk/Source/WebCore/ChangeLog 2014-03-24 23:11:43 UTC (rev 166202)
@@ -1,3 +1,21 @@
+2014-03-24 Daniel Bates <[email protected]>
+
+ XSS Auditor doesn't block <script> injected before an existing <script>
+ https://bugs.webkit.org/show_bug.cgi?id=130475
+
+ Merged from Blink (patch by Tom Sepez):
+ https://src.chromium.org/viewvc/blink?view=rev&revision=169697
+
+ Tests: http/tests/security/xssAuditor/script-tag-_expression_-follows.html
+ http/tests/security/xssAuditor/script-tag-near-start.html
+
+ * html/parser/XSSAuditor.cpp:
+ (WebCore::startsHTMLCommentAt):
+ (WebCore::startsSingleLineCommentAt):
+ (WebCore::startsMultiLineCommentAt):
+ (WebCore::startsOpeningScriptTagAt):
+ (WebCore::XSSAuditor::decodedSnippetForJavaScript):
+
2014-03-24 Brent Fulgham <[email protected]>
Activate WebVTT Tests Once Merging is Complete
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (166201 => 166202)
--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2014-03-24 23:01:56 UTC (rev 166201)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2014-03-24 23:11:43 UTC (rev 166202)
@@ -41,6 +41,7 @@
#include "Settings.h"
#include "TextResourceDecoder.h"
#include "XLinkNames.h"
+#include <wtf/ASCIICType.h>
#include <wtf/MainThread.h>
namespace WebCore {
@@ -87,19 +88,30 @@
static bool startsHTMLCommentAt(const String& string, size_t start)
{
- return (start + 3 < string.length() && string[start] == '<' && string[start+1] == '!' && string[start+2] == '-' && string[start+3] == '-');
+ return (start + 3 < string.length() && string[start] == '<' && string[start + 1] == '!' && string[start + 2] == '-' && string[start + 3] == '-');
}
static bool startsSingleLineCommentAt(const String& string, size_t start)
{
- return (start + 1 < string.length() && string[start] == '/' && string[start+1] == '/');
+ return (start + 1 < string.length() && string[start] == '/' && string[start + 1] == '/');
}
static bool startsMultiLineCommentAt(const String& string, size_t start)
{
- return (start + 1 < string.length() && string[start] == '/' && string[start+1] == '*');
+ return (start + 1 < string.length() && string[start] == '/' && string[start + 1] == '*');
}
+static bool startsOpeningScriptTagAt(const String& string, size_t start)
+{
+ return start + 6 < string.length() && string[start] == '<'
+ && WTF::toASCIILowerUnchecked(string[start + 1]) == 's'
+ && WTF::toASCIILowerUnchecked(string[start + 2]) == 'c'
+ && WTF::toASCIILowerUnchecked(string[start + 3]) == 'r'
+ && WTF::toASCIILowerUnchecked(string[start + 4]) == 'i'
+ && WTF::toASCIILowerUnchecked(string[start + 5]) == 'p'
+ && WTF::toASCIILowerUnchecked(string[start + 6]) == 't';
+}
+
// If other files need this, we should move this to HTMLParserIdioms.h
template<size_t inlineCapacity>
bool threadSafeMatch(const Vector<UChar, inlineCapacity>& vector, const QualifiedName& qname)
@@ -621,6 +633,7 @@
size_t startPosition = 0;
size_t endPosition = string.length();
size_t foundPosition = notFound;
+ size_t lastNonSpacePosition = notFound;
// Skip over initial comments to find start of code.
while (startPosition < endPosition) {
@@ -649,13 +662,10 @@
String result;
while (startPosition < endPosition && !result.length()) {
- // Stop at next comment (using the same rules as above for SVG/XML vs HTML), when we
- // encounter a comma, or when we exceed the maximum length target. The comma rule
- // covers a common parameter concatenation case performed by some webservers.
- // After hitting the length target, we can only stop at a point where we know we are
- // not in the middle of a %-escape sequence. For the sake of simplicity, approximate
- // not stopping inside a (possibly multiply encoded) %-esacpe sequence by breaking on
- // whitespace only. We should have enough text in these cases to avoid false positives.
+ // Stop at next comment (using the same rules as above for SVG/XML vs HTML), when we encounter a comma,
+ // when we hit an opening <script> tag, or when we exceed the maximum length target. The comma rule
+ // covers a common parameter concatenation case performed by some web servers.
+ lastNonSpacePosition = notFound;
for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
if (!request.shouldAllowCDATA) {
if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
@@ -667,9 +677,25 @@
break;
}
}
- if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
+ if (string[foundPosition] == ',')
break;
+
+ if (lastNonSpacePosition != notFound && startsOpeningScriptTagAt(string, foundPosition)) {
+ foundPosition = lastNonSpacePosition;
+ break;
}
+
+ if (foundPosition > startPosition + kMaximumFragmentLengthTarget) {
+ // After hitting the length target, we can only stop at a point where we know we are
+ // not in the middle of a %-escape sequence. For the sake of simplicity, approximate
+ // not stopping inside a (possibly multiply encoded) %-escape sequence by breaking on
+ // whitespace only. We should have enough text in these cases to avoid false positives.
+ if (isHTMLSpace(string[foundPosition]))
+ break;
+ }
+
+ if (!isHTMLSpace(string[foundPosition]))
+ lastNonSpacePosition = foundPosition;
}
result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding);
