Bug 81948

Summary: XSS Auditor bypass via script tag src=data:, URLS.
Product: WebKit Reporter: Thomas Sepez <tsepez>
Component: WebCore Misc.Assignee: Thomas Sepez <tsepez>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dbates, webkit.review.bot
Priority: P2 Keywords: XSSAuditor
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Testcase
none
Patch.
abarth: review+
Patch + style nits. none

Description Thomas Sepez 2012-03-22 12:18:09 PDT 
Originally reported by sirdarckcat at http://code.google.com/p/chromium/issues/detail?id=117329

What steps will reproduce the problem?
1. Go to http://0x.lv/xss.php?html_xss=%3Cscript%20src=%22data:,alert(1)//
Comment 1 Thomas Sepez 2012-03-22 12:18:16 PDT 
A minimized returned page for this looks like:

<html xmlns="http://www.w3.org/1999/xhtml">
<body>
<div class="lol">
<script src="data:,alert(1)//                                                   
<h1>existing page clutter</h1>                                                                
<script type="text/javascript">x = 2;</script>                                  
</body>                                                                         
</html>
Comment 2 Thomas Sepez 2012-03-22 12:26:59 PDT 
Created attachment 133323 [details]
Testcase
Comment 3 Thomas Sepez 2012-03-22 16:14:31 PDT 
Created attachment 133379 [details]
Patch.
Comment 4 Adam Barth 2012-03-22 16:46:17 PDT 
Comment on attachment 133379 [details]
Patch.

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

> Source/WebCore/html/parser/XSSAuditor.cpp:521
> +        bool commaSeen;

This is a personal preference, but I think it's better to initialize scalars when they're declared (and then to have an empty first-clause in the for statement).

> Source/WebCore/html/parser/XSSAuditor.cpp:530
>              if (decodedSnippet[currentLength] == '?' || decodedSnippet[currentLength] == '#'

At this point, I would store decodedSnippet[currentLength] in a local variable.
Comment 5 Thomas Sepez 2012-03-22 17:07:25 PDT 
Created attachment 133391 [details]
Patch + style nits.
Comment 6 WebKit Review Bot 2012-03-22 19:13:42 PDT 
Comment on attachment 133391 [details]
Patch + style nits.

Clearing flags on attachment: 133391

Committed r111808: <http://trac.webkit.org/changeset/111808>
Comment 7 WebKit Review Bot 2012-03-22 19:13:52 PDT 
All reviewed patches have been landed.  Closing bug.