Title: [135299] trunk
- Revision
- 135299
- Author
- tse...@chromium.org
- Date
- 2012-11-20 10:49:07 -0800 (Tue, 20 Nov 2012)
Log Message
XSSAuditor::decodedSnippetForJavaScript stopping when comma encountered.
https://bugs.webkit.org/show_bug.cgi?id=102587
Reviewed by Adam Barth.
Source/WebCore:
Rather than returning an empty fragment, continue processing the body
of a script tag when the decoded fragment reduces to nothing.
Test: http/tests/security/xssAuditor/script-tag-with-actual-comma.html
* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::decodedSnippetForJavaScript):
LayoutTests:
* http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt: Added.
* http/tests/security/xssAuditor/script-tag-with-actual-comma.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (135298 => 135299)
--- trunk/LayoutTests/ChangeLog 2012-11-20 18:46:35 UTC (rev 135298)
+++ trunk/LayoutTests/ChangeLog 2012-11-20 18:49:07 UTC (rev 135299)
@@ -1,3 +1,13 @@
+2012-11-20 Tom Sepez <tse...@chromium.org>
+
+ XSSAuditor::decodedSnippetForJavaScript stopping when comma encountered.
+ https://bugs.webkit.org/show_bug.cgi?id=102587
+
+ Reviewed by Adam Barth.
+
+ * http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt: Added.
+ * http/tests/security/xssAuditor/script-tag-with-actual-comma.html: Added.
+
2012-11-20 Hans Muller <hmul...@adobe.com>
[CSS Exclusions] writing-mode:vertical-rl shape-inside tests are incorrectly configured
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt (0 => 135299)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt 2012-11-20 18:49:07 UTC (rev 135299)
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: Refused to execute a _javascript_ script. Source code of script found within request.
+
+
+Test that the XSSAuditor's tolerance for the IIS webserver's comma concatenation doesn't open holes when the reflected argument contains an actual comma. The test passes if the XSSAuditor logs console messages and no alerts fire.
Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html (0 => 135299)
--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html 2012-11-20 18:49:07 UTC (rev 135299)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=""
+</iframe>
+<p>Test that the XSSAuditor's tolerance for the IIS webserver's comma concatenation doesn't open holes when the reflected argument
+contains an actual comma. The test passes if the XSSAuditor logs console messages and no alerts fire.</p>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (135298 => 135299)
--- trunk/Source/WebCore/ChangeLog 2012-11-20 18:46:35 UTC (rev 135298)
+++ trunk/Source/WebCore/ChangeLog 2012-11-20 18:49:07 UTC (rev 135299)
@@ -1,3 +1,18 @@
+2012-11-20 Tom Sepez <tse...@chromium.org>
+
+ XSSAuditor::decodedSnippetForJavaScript stopping when comma encountered.
+ https://bugs.webkit.org/show_bug.cgi?id=102587
+
+ Reviewed by Adam Barth.
+
+ Rather than returning an empty fragment, continue processing the body
+ of a script tag when the decoded fragment reduces to nothing.
+
+ Test: http/tests/security/xssAuditor/script-tag-with-actual-comma.html
+
+ * html/parser/XSSAuditor.cpp:
+ (WebCore::XSSAuditor::decodedSnippetForJavaScript):
+
2012-11-20 James Simonsen <simon...@chromium.org>
Consolidate FrameLoader::load() into one function taking a FrameLoadRequest
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (135298 => 135299)
--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2012-11-20 18:46:35 UTC (rev 135298)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2012-11-20 18:49:07 UTC (rev 135299)
@@ -614,31 +614,35 @@
break;
}
- // 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)) {
- endPosition = foundPosition + 2;
- break;
+ 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.
+ for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
+ if (!m_shouldAllowCDATA) {
+ if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
+ foundPosition += 2;
+ break;
+ }
+ if (startsHTMLCommentAt(string, foundPosition)) {
+ foundPosition += 4;
+ break;
+ }
}
- if (startsHTMLCommentAt(string, foundPosition)) {
- endPosition = foundPosition + 4;
+ if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
break;
}
}
- if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
- endPosition = foundPosition;
- break;
- }
+
+ result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_parser->document()->decoder());
+ startPosition = foundPosition + 1;
}
-
- return fullyDecodeString(string.substring(startPosition, endPosition - startPosition), m_parser->document()->decoder());
+ return result;
}
bool XSSAuditor::isContainedInRequest(const String& decodedSnippet)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
