Bug 84158

Summary: XSS Auditor bypass via svg tags and xlink:href
Product: WebKit Reporter: Thomas Sepez <tsepez>
Component: WebCore Misc.Assignee: Thomas Sepez <tsepez>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dbates, ojan.autocc, webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
testcase
none
Patch.
webkit-ews: commit-queue-
Patch, fix qt build.
abarth: review+, buildbot: commit-queue-
Patch, check mixed case in test. none

Attachments
testcase (1.11 KB, patch)
2013-02-01 13:14 PST, Thomas Sepez 		no flags
DetailsFormatted DiffDiff
Patch. (4.57 KB, patch)
2013-02-01 15:51 PST, Thomas Sepez 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
Patch, fix qt build. (4.58 KB, patch)
2013-02-01 16:10 PST, Thomas Sepez 		abarth: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
Patch, check mixed case in test. (4.58 KB, patch)
2013-02-04 10:00 PST, Thomas Sepez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Thomas Sepez
Comment 1 2013-01-31 15:18:15 PST
Looks like we never fixed this one. Was there an issue with (like this not going through the HTML parser) that made this intractible?
Adam Barth
Comment 2 2013-02-01 00:10:12 PST
Seems like it should be easily fixable.
Thomas Sepez
Comment 3 2013-02-01 13:14:55 PST
Created attachment 186118 [details] testcase
Thomas Sepez
Comment 4 2013-02-01 15:51:57 PST
Created attachment 186174 [details] Patch.
Early Warning System Bot
Comment 5 2013-02-01 16:02:57 PST
Comment on attachment 186174 [details] Patch. Attachment 186174 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16332286
Thomas Sepez
Comment 6 2013-02-01 16:10:31 PST
Created attachment 186181 [details] Patch, fix qt build.
Adam Barth
Comment 7 2013-02-02 00:08:04 PST
Comment on attachment 186181 [details] Patch, fix qt build. View in context: https://bugs.webkit.org/attachment.cgi?id=186181&action=review > Source/WebCore/html/parser/XSSAuditor.cpp:122 > + String attrName(name.localName().string()); I would just use the assignment form of the constructor. > Source/WebCore/html/parser/XSSAuditor.cpp:125 > + if (name.namespaceURI() == XLinkNames::xlinkNamespaceURI) > + attrName = "xlink:" + attrName; Is it possible to use a different prefix than "xlink"? What about "xLinK:" ?
Build Bot
Comment 8 2013-02-02 01:20:24 PST
Comment on attachment 186181 [details] Patch, fix qt build. Attachment 186181 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16341600
Thomas Sepez
Comment 9 2013-02-04 09:55:20 PST
(In reply to comment #7) > (From update of attachment 186181 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186181&action=review > > > Source/WebCore/html/parser/XSSAuditor.cpp:122 > > + String attrName(name.localName().string()); > > I would just use the assignment form of the constructor. Done. > > > Source/WebCore/html/parser/XSSAuditor.cpp:125 > > + if (name.namespaceURI() == XLinkNames::xlinkNamespaceURI) > > + attrName = "xlink:" + attrName; > > Is it possible to use a different prefix than "xlink"? What about "xLinK:" ? Updated test to say XLink:, and this is getting converted to lowercase somewhere along the way.
Thomas Sepez
Comment 10 2013-02-04 10:00:28 PST
Created attachment 186406 [details] Patch, check mixed case in test.
WebKit Review Bot
Comment 11 2013-02-04 11:50:17 PST
Comment on attachment 186406 [details] Patch, check mixed case in test. Clearing flags on attachment: 186406 Committed r141791: <http://trac.webkit.org/changeset/141791>
WebKit Review Bot
Comment 12 2013-02-04 11:50:22 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.