Commit fd68a9fe authored by tsepez@chromium.org's avatar tsepez@chromium.org

This is another corner case where clutter from the page may prevent the

XSSAuditor from performing a proper match. Compare attr="subvalue;subvalue;clutter"
on a per-subvalue basis for the first suspicious subvalue.

BUG=384077

Review URL: https://codereview.chromium.org/337143004

git-svn-id: svn://svn.chromium.org/blink/trunk@176359 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 0f4ee921
CONSOLE ERROR: line 4: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=%3Csvg%20xmlns:xlink=%27http://www.w3.org/1999/xlink%27%3E%3Ca%3E%3Ccircle%20r=100%20/%3E%3Canimate%20attributeName=xlink:href%20values=%3Bjavascript%3Aalert(1)%3B&clutter=blah%27%3E&notifyDone=1&dumpElementBySelector=animate' 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.
This test passes if the element displayed in the frame below has a 'values' attribute containing only 'javascript:void(0)'.
--------
Frame: '<!--framePath //<!--frame0-->-->'
--------
animate => animate
* attributeName: xlink:href
* values: javascript:void(0)
<!DOCTYPE html>
<html>
<head>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
testRunner.waitUntilDone();
testRunner.setXSSAuditorEnabled(true);
}
</script>
</head>
<body>
<p>This test passes if the element displayed in the frame below has a 'values' attribute containing only 'javascript:void(0)'.</p>
<iframe src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<svg%20xmlns:xlink='http://www.w3.org/1999/xlink'><a><circle%20r=100%20/><animate%20attributeName=xlink:href%20values=%3Bjavascript%3Aalert(1)%3B&clutter=blah'>&notifyDone=1&dumpElementBySelector=animate"></iframe>
</body>
</html>
......@@ -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.
......
......@@ -103,6 +103,7 @@ private:
String canonicalizedSnippetForTagName(const FilterTokenRequest&);
String canonicalizedSnippetForJavaScript(const FilterTokenRequest&);
String nameFromAttribute(const FilterTokenRequest&, const HTMLToken::Attribute&);
String snippetFromAttribute(const FilterTokenRequest&, const HTMLToken::Attribute&);
String canonicalize(String, TruncationKind);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment