Bug 79154 - [chromium] XSS Auditor bypass via javascript url and control characters
Summary: [chromium] XSS Auditor bypass via javascript url and control characters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thomas Sepez
URL:
Keywords: XSSAuditor
Depends on:
Blocks:
 
Reported: 2012-02-21 15:28 PST by Thomas Sepez
Modified: 2012-02-23 11:46 PST (History)
2 users (show)

See Also:

Attachments
Patch. (4.07 KB, patch)
2012-02-21 15:37 PST, Thomas Sepez 		abarth: review-
Details | Formatted Diff | Diff
Patch. (5.19 KB, patch)
2012-02-22 17:42 PST, Thomas Sepez 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Sepez 2012-02-21 15:28:49 PST 
Reported by masatokinugawa, Today (10 hours ago)
VULNERABILITY DETAILS
The vector is:

?xss=1&body=%3Ca%20href=%22ja%09vascript:alert(1)%22%3EClick%20here%3C/a%3E

Safari is OK.

Also Reported by kuzzcc, Yesterday (32 hours ago)
Chrome 19.0.1041.0 dev-m window xp sp3

?q=%3Ca%20href=%22%26%231;javascript:alert(0)%22%3Etest%3C/a%3E
data:text/html,<a href="&#1;javascript:alert(0)">test</a>
Comment 1 Thomas Sepez 2012-02-21 15:37:00 PST 
Created attachment 128061 [details]
Patch.
Comment 2 Adam Barth 2012-02-21 16:05:03 PST 
Comment on attachment 128061 [details]
Patch.

This isn't the right approach...  There should be a function already that does this work called something like protocolIs or protocolIsJavaScript...
Comment 3 Adam Barth 2012-02-21 16:06:09 PST 
Take a look at allowSettingJavaScriptURL in JSHTMLFrameElementCustom.cpp, for example.
Comment 4 Adam Barth 2012-02-21 16:06:45 PST 
allowSettingSrcToJavaScriptUrl
Comment 5 Thomas Sepez 2012-02-22 12:11:53 PST 
allowSettingFrameSrcToJavascriptUrl() does the check:

    if (protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(value))) { ...

If I'm reading this right, stripLeadingAndTrailingHTMLSpaces() [HTMLParserIdioms.cpp] doesn't remove control-characters or internal HTML spaces.  protocolIsJavaScript()  [KURL.cpp] uses protocolIs() which asserts on c <= 0x20 in DEBUG builds but nothing otherwise.  So it looks like this check is hosed in the same was as XSSauditor if the (specific to chromium, right?) code allows &#1; and the like --  Unless there is a higher-level check.

Flipping to component security while I cobble together an example see if there's actually a  hole here.
Comment 6 Thomas Sepez 2012-02-22 14:37:07 PST 
Ah.  KURL vs. KURLgoogle again.   KURLgoogle's protocolIs() will handle the ctrl characters, so no UXSS hole on setting frame.location.    Good.  

So we can use protocolIsJavascript(), but test will need to be chromium-only.
Comment 7 Thomas Sepez 2012-02-22 17:42:34 PST 
Created attachment 128349 [details]
Patch.

Patch using Adam's suggested function.  Also, the test now uses an <a href=""> rather than an <iframe src=""> since iframe src currently isn't exploitable.  It flunks an origin test on chromium -- which is why this needs to be href in an a tag.  Still waiting full testing completion.
Comment 8 WebKit Review Bot 2012-02-23 11:46:35 PST 
Comment on attachment 128349 [details]
Patch.

Clearing flags on attachment: 128349

Committed r108653: <http://trac.webkit.org/changeset/108653>
Comment 9 WebKit Review Bot 2012-02-23 11:46:42 PST 
All reviewed patches have been landed.  Closing bug.