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:

Keywords:

XSSAuditor

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Testcase	none
Patch.	abarth: review+
Patch + style nits.	none

Thomas Sepez

Reported 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)//

Attachments
Testcase (371 bytes, text/plain) 2012-03-22 12:26 PDT, Thomas Sepez	no flags	Details
Patch. (5.16 KB, patch) 2012-03-22 16:14 PDT, Thomas Sepez	abarth: review+	Details Formatted Diff Diff
Patch + style nits. (5.41 KB, patch) 2012-03-22 17:07 PDT, Thomas Sepez	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Thomas Sepez

Comment 1 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>

Thomas Sepez

Comment 2 2012-03-22 12:26:59 PDT

Created attachment 133323 [details] Testcase

Thomas Sepez

Comment 3 2012-03-22 16:14:31 PDT

Created attachment 133379 [details] Patch.

Adam Barth

Comment 4 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.

Thomas Sepez

Comment 5 2012-03-22 17:07:25 PDT

Created attachment 133391 [details] Patch + style nits.

WebKit Review Bot

Comment 6 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>

WebKit Review Bot

Comment 7 2012-03-22 19:13:52 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.