RESOLVED FIXED 78836
XSSAuditor bypass with <svg> tags and html-entities.
https://bugs.webkit.org/show_bug.cgi?id=78836
Summary XSSAuditor bypass with <svg> tags and html-entities.
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
Patch for initial review (needs more testing before commit). (22.85 KB, patch)
2012-02-21 13:53 PST, Thomas Sepez
abarth: review+
Patch + indent fix. (22.85 KB, patch)
2012-02-22 12:54 PST, Thomas Sepez
no flags
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.