Bug 78836

Summary:

XSSAuditor bypass with <svg> tags and html-entities.

Product:

WebKit

Reporter:

Thomas Sepez <tsepez>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dbates, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
First cut at Patch -- needs more analysis.	none
Patch for initial review (needs more testing before commit).	abarth: review+
Patch + indent fix.	none

Thomas Sepez

Reported 2012-02-16 13:35:03 PST

Originally reported by masatokinugawa at http://code.google.com/p/chromium/issues/detail?id=114641 Reflected vectors are of the form: ?xss=%3Csvg%3E%3Cscript%3E//%26%23x0A%3Balert(1)%3C/script%3E ?xss=%3Csvg%3E%3Cscript%3E//%26%23x0D%3Balert(1)%3C/script%3E ?xss=%3Csvg%3E%3Cscript%3E//%26%23x2028%3Balert(1)%3C/script%3E ?xss=%3Csvg%3E%3Cscript%3E//%26%23x2029%3Balert(1)%3C/script%3E ?xss=%3Csvg%3E%3Cscript%3E/**%26%23x2F%3Balert(1)%3C/script%3E SVG is XML and hence the entities get decoded during parsing.

Attachments
First cut at Patch -- needs more analysis. (17.90 KB, patch) 2012-02-17 16:19 PST, Thomas Sepez	no flags	Details Formatted Diff Diff
Patch for initial review (needs more testing before commit). (22.85 KB, patch) 2012-02-21 13:53 PST, Thomas Sepez	abarth: review+	Details Formatted Diff Diff
Patch + indent fix. (22.85 KB, patch) 2012-02-22 12:54 PST, Thomas Sepez	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Thomas Sepez

Comment 1 2012-02-16 13:51:12 PST

Probably the right way to fix this is for xssauditor to know when it is in an SVG block vs. an ordinary script block, and apply html entity decoding to match the HTML vs. XML expectations. Trying to always html entity decode will open up vulns in the HTML case.

Thomas Sepez

Comment 2 2012-02-17 16:19:58 PST

Created attachment 127672 [details] First cut at Patch -- needs more analysis.

Thomas Sepez

Comment 3 2012-02-21 13:53:38 PST

Created attachment 128037 [details] Patch for initial review (needs more testing before commit).

Thomas Sepez

Comment 4 2012-02-21 14:03:23 PST

So the idea in patch #2 is that the state of initial token / after scrip tag goes away, and is replaced by a count of the depth of the script blocks (which is cheaper than trying to keep a stack of tags).

Adam Barth

Comment 5 2012-02-21 14:19:55 PST

Comment on attachment 128037 [details] Patch for initial review (needs more testing before commit). View in context: https://bugs.webkit.org/attachment.cgi?id=128037&action=review This looks fantastic. Thanks. > Source/WebCore/html/parser/XSSAuditor.cpp:586 > + break; bad indent

Thomas Sepez

Comment 6 2012-02-22 12:54:14 PST

Created attachment 128269 [details] Patch + indent fix. Let's try to land this to avoid merging around it.

WebKit Review Bot

Comment 7 2012-02-22 14:21:00 PST

Comment on attachment 128269 [details] Patch + indent fix. Clearing flags on attachment: 128269 Committed r108551: <http://trac.webkit.org/changeset/108551>

WebKit Review Bot

Comment 8 2012-02-22 14:21:05 PST

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.