xssauditor - bypass with unterminated closing script tag

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Thomas Sepez]

Upstreamed from http://code.google.com/p/chromium/issues/detail?id=96845

Reported by [e-mail], Today (5 hours ago)

VULNERABILITY DETAILS
It is possible to bypass Google Chrome's Anti-XSS filtering
mechanism through the use of a specially crafted parameter.
The parameter, which is initially not valid JavaScript, passes
through the filter and then gets 'corrected' by Chrome, turning it
into valid JavaScript. Please do not confuse with the other 
bypass that needs two parameters
(http://code.google.com/p/chromium/issues/detail?id=96616).

REPRODUCTION CASE
E.g. 

http://securitee.org/files/chrome_xss_again.php?a=<script id=<script>alert(1)</script

In the above case, there is no closing ">" after the value of the id and no closing ">" at the script closing tag, making the whole thing invalid. However, Chrome will happily turn this internally into:

http://securitee.org/files/chrome_xss_again.php?a=<script id="<script">alert(1)</script>


Reduction:

http://securitee.org/files/chrome_xss_again.php?a=%3Cscript%3Ealert(1)%3C/script

[Thomas Sepez]

Created attachment 107887 [details]
Patch to set end location of token before additional buffering takes place.

[Adam Barth]

Comment on attachment 107887 [details]
Patch to set end location of token before additional buffering takes place.

View in context: https://bugs.webkit.org/attachment.cgi?id=107887&action=review

> Source/WebCore/html/parser/HTMLTokenizer.cpp:305
> -        if (cc == '<')
> +        if (cc == '<') {
> +            // Token might end here. If not, we'll come through here again
> +            // and update the end location again.
> +            m_token->end(source.numberOfCharactersConsumed());
>              HTML_ADVANCE_TO(ScriptDataLessThanSignState);
> +        }

Interesting.  We have this same problem for CDATA and RCDATA.  For example, the <title> and the <style> tags.  It would be good to apply this kind of fix in those cases too, maybe in a follow-up patch.

This patch feels a little bit like a hack because we're only doing this in one case, but I do agree that this patch is moving us in the right direction because the tokenizer should be setting the end marker for the token.

[WebKit Review Bot]

Comment on attachment 107887 [details]
Patch to set end location of token before additional buffering takes place.

Clearing flags on attachment: 107887

Committed r95451: <http://trac.webkit.org/changeset/95451>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.