Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Unified Diff: Source/core/html/parser/XSSAuditor.cpp

Issue 205243002: XSSAuditor bypass with script tag and expression following injection point (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Incorporate dbates's suggestions. Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « LayoutTests/http/tests/security/xssAuditor/script-tag-near-start-expected.txt ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/html/parser/XSSAuditor.cpp
diff --git a/Source/core/html/parser/XSSAuditor.cpp b/Source/core/html/parser/XSSAuditor.cpp
index 7adae45adddac45258c9ca04b8bbbca3c74e71ae..05ae9cbafb937d7d06da5945cd787d34d91a3af9 100644
--- a/Source/core/html/parser/XSSAuditor.cpp
+++ b/Source/core/html/parser/XSSAuditor.cpp
@@ -43,6 +43,7 @@
#include "platform/JSONValues.h"
#include "platform/network/FormData.h"
#include "platform/text/DecodeEscapeSequences.h"
+#include "wtf/ASCIICType.h"
#include "wtf/MainThread.h"
namespace {
@@ -98,17 +99,28 @@ static bool isJSNewline(UChar c)
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 core/html/parser/HTMLParserIdioms.h
@@ -649,6 +661,7 @@ String XSSAuditor::decodedSnippetForJavaScript(const FilterTokenRequest& request
size_t startPosition = 0;
size_t endPosition = string.length();
size_t foundPosition = kNotFound;
+ size_t lastNonSpacePosition = kNotFound;
// Skip over initial comments to find start of code.
while (startPosition < endPosition) {
@@ -677,13 +690,10 @@ String XSSAuditor::decodedSnippetForJavaScript(const FilterTokenRequest& request
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 = kNotFound;
for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
if (!request.shouldAllowCDATA) {
if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
@@ -695,9 +705,25 @@ String XSSAuditor::decodedSnippetForJavaScript(const FilterTokenRequest& request
break;
}
}
- if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace<UChar>(string[foundPosition]))) {
+ if (string[foundPosition] == ',')
+ break;
+
+ if (lastNonSpacePosition != kNotFound && 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<UChar>(string[foundPosition]))
+ break;
+ }
+
+ if (!isHTMLSpace<UChar>(string[foundPosition]))
+ lastNonSpacePosition = foundPosition;
}
result = canonicalize(fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding));
« no previous file with comments | « LayoutTests/http/tests/security/xssAuditor/script-tag-near-start-expected.txt ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698