Skip to content

GitLab

T
tangled
Commit 0844102c authored by tsepez@chromium.org's avatar tsepez@chromium.org
Browse files
Options

XSSAuditor: treat questionmark as a non-canonical character. 

We've seen recent examples of servers that replace an invalid set of
of high-bytes with a literal questionmark.  We are already excluding
the high bytes from consideration, so we do the same with the
questionmark to ensure a match should this happen.

To test this, we hack up our "server script" to replace an arbitrary high
byte with a questionmark. This is sufficient for testing although it may
not match any real server.

BUG=395351

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

git-svn-id: svn://svn.chromium.org/blink/trunk@179161 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 04ac10bf
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 3 deletions
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl
View file @ 0844102c
...@@ -92,7 +92,11 @@ if ($cgi->param('inHead')) { ...@@ -92,7 +92,11 @@ if ($cgi->param('inHead')) {
if ($cgi->param('replaceState')) { if ($cgi->param('replaceState')) {
print "<script>history.replaceState({}, '', '#must-not-appear');</script>\n"; print "<script>history.replaceState({}, '', '#must-not-appear');</script>\n";
} }
print $cgi->param('q'); # XSS reflected here. my $reflection = $cgi->param('q');
my $pattern = "\xfe";
my $replacement = "?";
$reflection =~ s/$pattern/$replacement/g; # pretend server translates high character 0xfe into literal "?".
print $reflection; # XSS reflected here.
if ($cgi->param('script-expression-follows')) { if ($cgi->param('script-expression-follows')) {
print "\n <script>42;</script>\n"; print "\n <script>42;</script>\n";
} }
......
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/script-tag-replaced-with-questionmark-expected.txt 0 → 100644
View file @ 0844102c
CONSOLE ERROR: line 4: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=%3Cscript%3Ealert(%22xxx%fexxx%22)%3C/script%3E' 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.
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/script-tag-replaced-with-questionmark.html 0 → 100644
View file @ 0844102c
<!DOCTYPE html>
<html>
<head>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.setXSSAuditorEnabled(true);
}
</script>
</head>
<body>
<iframe src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<script>alert(%22xxx%fexxx%22)</script>">
</iframe>
</body>
</html>
third_party/WebKit/Source/core/html/parser/XSSAuditor.cpp
View file @ 0844102c
...@@ -64,11 +64,15 @@ static bool isNonCanonicalCharacter(UChar c) ...@@ -64,11 +64,15 @@ static bool isNonCanonicalCharacter(UChar c)
// Note, we don't remove backslashes like PHP stripslashes(), which among other things converts "\\0" to the \0 character. // Note, we don't remove backslashes like PHP stripslashes(), which among other things converts "\\0" to the \0 character.
// Instead, we remove backslashes and zeros (since the string "\\0" =(remove backslashes)=> "0"). However, this has the // Instead, we remove backslashes and zeros (since the string "\\0" =(remove backslashes)=> "0"). However, this has the
// adverse effect that we remove any legitimate zeros from a string. // adverse effect that we remove any legitimate zeros from a string.
//
// We also remove forward-slash, because it is common for some servers to collapse successive path components, eg, // We also remove forward-slash, because it is common for some servers to collapse successive path components, eg,
// a//b becomes a/b. // a//b becomes a/b.
// //
// For instance: new String("http://localhost:8000") => new String("http:localhost:8"). // We also remove the questionmark character, since some severs replace invalid high-bytes with a questionmark. We
return (c == '\\' || c == '0' || c == '\0' || c == '/' || c >= 127); // are already stripping the high-bytes so we also strip the questionmark to match.
//
// For instance: new String("http://localhost:8000?x") => new String("http:localhost:8x").
return (c == '\\' || c == '0' || c == '\0' || c == '/' || c == '?' || c >= 127);
} }
static bool isRequiredForInjection(UChar c) static bool isRequiredForInjection(UChar c)
......
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