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

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

Issue 337143004: Fix XSSAuditor handling of semicolon-separated attributes. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Use emptyString() Created 6 years, 6 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 | « Source/core/html/parser/XSSAuditor.h ('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 77912acd33767a4976ba3af27f196ca9fd1d7c02..bef0b2043468ac4f10f94cc2af7824f77a1884d8 100644
--- a/Source/core/html/parser/XSSAuditor.cpp
+++ b/Source/core/html/parser/XSSAuditor.cpp
@@ -249,15 +249,16 @@ static bool isSemicolonSeparatedAttribute(const HTMLToken::Attribute& attribute)
return threadSafeMatch(attribute.name, SVGNames::valuesAttr);
}
-static bool semicolonSeparatedValueContainsJavaScriptURL(const String& value)
+static String semicolonSeparatedValueContainingJavaScriptURL(const String& value)
{
Vector<String> valueList;
value.split(';', valueList);
for (size_t i = 0; i < valueList.size(); ++i) {
- if (protocolIsJavaScript(valueList[i]))
- return true;
+ String stripped = stripLeadingAndTrailingHTMLSpaces(valueList[i]);
+ if (protocolIsJavaScript(stripped))
+ return stripped;
}
- return false;
+ return emptyString();
}
XSSAuditor::XSSAuditor()
@@ -600,14 +601,24 @@ bool XSSAuditor::eraseDangerousAttributesIfInjected(const FilterTokenRequest& re
bool didBlockScript = false;
for (size_t i = 0; i < request.token.attributes().size(); ++i) {
+ bool eraseAttribute = false;
+ bool valueContainsJavaScriptURL = false;
const HTMLToken::Attribute& attribute = request.token.attributes().at(i);
- bool isInlineEventHandler = isNameOfInlineEventHandler(attribute.name);
- // FIXME: It would be better if we didn't create a new String for every attribute in the document.
- String strippedValue = stripLeadingAndTrailingHTMLSpaces(String(attribute.value));
- bool valueContainsJavaScriptURL = (!isInlineEventHandler && protocolIsJavaScript(strippedValue)) || (isSemicolonSeparatedAttribute(attribute) && semicolonSeparatedValueContainsJavaScriptURL(strippedValue));
- if (!isInlineEventHandler && !valueContainsJavaScriptURL)
- continue;
- if (!isContainedInRequest(canonicalize(snippetFromAttribute(request, attribute), ScriptLikeAttributeTruncation)))
+ // FIXME: Don't create a new String for every attribute.value in the document.
+ if (isNameOfInlineEventHandler(attribute.name)) {
+ eraseAttribute = isContainedInRequest(canonicalize(snippetFromAttribute(request, attribute), ScriptLikeAttributeTruncation));
+ } else if (protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(String(attribute.value)))) {
+ valueContainsJavaScriptURL = true;
+ eraseAttribute = isContainedInRequest(canonicalize(snippetFromAttribute(request, attribute), ScriptLikeAttributeTruncation));
+ } else if (isSemicolonSeparatedAttribute(attribute)) {
+ String subValue = semicolonSeparatedValueContainingJavaScriptURL(String(attribute.value));
+ if (!subValue.isEmpty()) {
+ valueContainsJavaScriptURL = true;
+ eraseAttribute = isContainedInRequest(canonicalize(nameFromAttribute(request, attribute), NoTruncation))
+ && isContainedInRequest(canonicalize(subValue, ScriptLikeAttributeTruncation));
+ }
+ }
+ if (!eraseAttribute)
continue;
request.token.eraseValueOfAttribute(i);
if (valueContainsJavaScriptURL)
@@ -648,9 +659,18 @@ String XSSAuditor::canonicalizedSnippetForTagName(const FilterTokenRequest& requ
return canonicalize(request.sourceTracker.sourceForToken(request.token).substring(0, request.token.name().size() + 1), NoTruncation);
}
+String XSSAuditor::nameFromAttribute(const FilterTokenRequest& request, const HTMLToken::Attribute& attribute)
+{
+ // The range inlcudes the character which terminates the name. So,
+ // for an input of |name="value"|, the snippet is |name=|.
+ int start = attribute.nameRange.start - request.token.startIndex();
+ int end = attribute.valueRange.start - request.token.startIndex();
+ return request.sourceTracker.sourceForToken(request.token).substring(start, end - start);
+}
+
String XSSAuditor::snippetFromAttribute(const FilterTokenRequest& request, const HTMLToken::Attribute& attribute)
{
- // The range doesn't inlcude the character which terminates the value. So,
+ // The range doesn't include the character which terminates the value. So,
// for an input of |name="value"|, the snippet is |name="value|. For an
// unquoted input of |name=value |, the snippet is |name=value|.
// FIXME: We should grab one character before the name also.
« no previous file with comments | « Source/core/html/parser/XSSAuditor.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698