- Revision
- 113263
- Author
- [email protected]
- Date
- 2012-04-04 16:54:28 -0700 (Wed, 04 Apr 2012)
Log Message
XSSAuditor bypass through HTTP Parameter Pollution.
https://bugs.webkit.org/show_bug.cgi?id=81283
Reviewed by Adam Barth.
Source/WebCore:
Deal with concatenation of multiple parameters via comma-splicing that
is common to some webservers. We can no longer trust that all of the
attributes of a reflected script tag, nor the reflected script itself,
came from the same single URL parameter. The fix is to take commas into
account when trucating the snippet used for matching.
Test: http/tests/security/xssAuditor/script-tag-with-comma.html
* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::filterScriptToken):
(WebCore):
(WebCore::XSSAuditor::decodedSnippetForName):
(WebCore::XSSAuditor::decodedSnippetForJavaScript):
LayoutTests:
* http/tests/security/xssAuditor/script-tag-with-comma-expected.txt: Added.
* http/tests/security/xssAuditor/script-tag-with-comma.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (113262 => 113263)
--- trunk/LayoutTests/ChangeLog 2012-04-04 23:50:53 UTC (rev 113262)
+++ trunk/LayoutTests/ChangeLog 2012-04-04 23:54:28 UTC (rev 113263)
@@ -1,3 +1,13 @@
+2012-04-04 Tom Sepez <[email protected]>
+
+ XSSAuditor bypass through HTTP Parameter Pollution.
+ https://bugs.webkit.org/show_bug.cgi?id=81283
+
+ Reviewed by Adam Barth.
+
+ * http/tests/security/xssAuditor/script-tag-with-comma-expected.txt: Added.
+ * http/tests/security/xssAuditor/script-tag-with-comma.html: Added.
+
2012-04-04 Dimitri Glazkov <[email protected]>
[Chromium] Update expectations for Leopard.
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma-expected.txt (0 => 113263)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma-expected.txt 2012-04-04 23:54:28 UTC (rev 113263)
@@ -0,0 +1,6 @@
+CONSOLE MESSAGE: Refused to execute a _javascript_ script. Source code of script found within request.
+
+CONSOLE MESSAGE: Refused to execute a _javascript_ script. Source code of script found within request.
+
+
+Test that the XSSAuditor catches the specific case where the IIS webserver resovles multiply occuring query parameters by concatenating them before passing the result to the application. Conceptually, its as if ?a=1&a=2 becomes ?a=1,2. The test passes if the XSSAuditor logs console messages and no alerts fire.
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma.html (0 => 113263)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-comma.html 2012-04-04 23:54:28 UTC (rev 113263)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=""
+<iframe src=""
+</iframe>
+<p>Test that the XSSAuditor catches the specific case where the IIS webserver resovles multiply occuring query parameters by
+concatenating them before passing the result to the application. Conceptually, its as if ?a=1&a=2 becomes ?a=1,2. The test
+passes if the XSSAuditor logs console messages and no alerts fire.</p>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (113262 => 113263)
--- trunk/Source/WebCore/ChangeLog 2012-04-04 23:50:53 UTC (rev 113262)
+++ trunk/Source/WebCore/ChangeLog 2012-04-04 23:54:28 UTC (rev 113263)
@@ -1,3 +1,24 @@
+2012-04-04 Tom Sepez <[email protected]>
+
+ XSSAuditor bypass through HTTP Parameter Pollution.
+ https://bugs.webkit.org/show_bug.cgi?id=81283
+
+ Reviewed by Adam Barth.
+
+ Deal with concatenation of multiple parameters via comma-splicing that
+ is common to some webservers. We can no longer trust that all of the
+ attributes of a reflected script tag, nor the reflected script itself,
+ came from the same single URL parameter. The fix is to take commas into
+ account when trucating the snippet used for matching.
+
+ Test: http/tests/security/xssAuditor/script-tag-with-comma.html
+
+ * html/parser/XSSAuditor.cpp:
+ (WebCore::XSSAuditor::filterScriptToken):
+ (WebCore):
+ (WebCore::XSSAuditor::decodedSnippetForName):
+ (WebCore::XSSAuditor::decodedSnippetForJavaScript):
+
2012-04-04 Dan Bernstein <[email protected]>
Paginated webviews render nothing in their gutters
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (113262 => 113263)
--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2012-04-04 23:50:53 UTC (rev 113262)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2012-04-04 23:54:28 UTC (rev 113263)
@@ -326,7 +326,7 @@
ASSERT(token.type() == HTMLTokenTypes::StartTag);
ASSERT(hasName(token, scriptTag));
- m_cachedDecodedSnippet = stripLeadingAndTrailingHTMLSpaces(decodedSnippetForToken(token));
+ m_cachedDecodedSnippet = decodedSnippetForName(token);
m_shouldAllowCDATA = m_parser->tokenizer()->shouldAllowCDATA();
if (isContainedInRequest(decodedSnippetForName(token)))
@@ -492,17 +492,10 @@
return false;
}
-String XSSAuditor::decodedSnippetForToken(const HTMLToken& token)
-{
- String snippet = m_parser->sourceForToken(token);
- return fullyDecodeString(snippet, m_parser->document()->decoder());
-}
-
String XSSAuditor::decodedSnippetForName(const HTMLToken& token)
{
- // Grab a fixed number of characters equal to the length of the token's
- // name plus one (to account for the "<").
- return decodedSnippetForToken(token).substring(0, token.name().size() + 1);
+ // Grab a fixed number of characters equal to the length of the token's name plus one (to account for the "<").
+ return fullyDecodeString(m_parser->sourceForToken(token), m_parser->document()->decoder()).substring(0, token.name().size() + 1);
}
String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute, AttributeKind treatment)
@@ -570,12 +563,13 @@
break;
}
- // Stop at next comment (using the same rules as above for SVG/XML vs HTML), or when
- // we exceed the maximum length target. 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, 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.
for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
if (!m_shouldAllowCDATA) {
if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
@@ -587,7 +581,7 @@
break;
}
}
- if (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition])) {
+ if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
endPosition = foundPosition;
break;
}
