Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVG data-URI image being removed #205

Closed
tabdon opened this issue Mar 11, 2017 · 10 comments
Closed

SVG data-URI image being removed #205

tabdon opened this issue Mar 11, 2017 · 10 comments

Comments

@tabdon
Copy link

tabdon commented Mar 11, 2017

Background & Context

I'm trying to cleanse the SVG output of a third-party diagramming tool. This tool draws some SVG elements using the <image> tag and xlink:href="data:image/svg+xml..." attribute.

Bug

DOMPurify strips out the xlink:href="data:image/svg+xml..." attribute completely. I've tried allowing xlink:href using the following statement, but it does not work:

var clean = DOMPurify.sanitize(dirty, {ADD_ATTR: ['xlink:href']});

Input

<image id="v-146" width="500" height="500" xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="data:image/svg+xml;utf8,%3Csvg%20viewBox%3D%220%200%20100%20100%22%20height%3D%22100%22%20width%3D%22100%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20data-name%3D%22Layer%201%22%20id%3D%22Layer_1%22%3E%0A%20%20%3Ctitle%3ECompute%3C%2Ftitle%3E%0A%20%20%3Cg%3E%0A%20%20%20%20%3Crect%20fill%3D%22%239d5025%22%20ry%3D%229.12%22%20rx%3D%229.12%22%20height%3D%2253%22%20width%3D%2253%22%20y%3D%2224.74%22%20x%3D%2223.5%22%3E%3C%2Frect%3E%0A%20%20%20%20%3Crect%20fill%3D%22%23f58536%22%20ry%3D%229.12%22%20rx%3D%229.12%22%20height%3D%2253%22%20width%3D%2253%22%20y%3D%2222.26%22%20x%3D%2223.5%22%3E%3C%2Frect%3E%0A%20%20%3C%2Fg%3E%0A%3C%2Fsvg%3E" preserveratio="true" style="border-color: rgb(51, 51, 51); box-sizing: border-box; color: rgb(51, 51, 51); cursor: move; font-family: sans-serif; font-size: 14px; line-height: 20px; outline-color: rgb(51, 51, 51); text-size-adjust: 100%; column-rule-color: rgb(51, 51, 51); -webkit-font-smoothing: antialiased; -webkit-tap-highlight-color: rgba(0, 0, 0, 0); -webkit-text-emphasis-color: rgb(51, 51, 51); -webkit-text-fill-color: rgb(51, 51, 51); -webkit-text-stroke-color: rgb(51, 51, 51); user-select: none; vector-effect: non-scaling-stroke;"></image>

Given output

<image style="border-color: rgb(51, 51, 51); box-sizing: border-box; color: rgb(51, 51, 51); cursor: move; font-family: sans-serif; font-size: 14px; line-height: 20px; outline-color: rgb(51, 51, 51); text-size-adjust: 100%; column-rule-color: rgb(51, 51, 51); -webkit-font-smoothing: antialiased; -webkit-tap-highlight-color: rgba(0, 0, 0, 0); -webkit-text-emphasis-color: rgb(51, 51, 51); -webkit-text-fill-color: rgb(51, 51, 51); -webkit-text-stroke-color: rgb(51, 51, 51); user-select: none; vector-effect: non-scaling-stroke;" height="500" width="500" id="v-146"></image>

Expected output

I'm trying to figure this part out. It sounds like the input code may cause XSS issues with Opera (per #148). So is the input code recommended, or should I transform it to something else?

@cure53
Copy link
Owner

cure53 commented Mar 13, 2017

This indeed causes XSS in older Opera versions (especially the releases from 9.x to 12.x) but then again, everything causes XSS in older Opera.

It is futile to attempt XSS protection for this train-wreck of a browser so I think we're okay to permit data URIs for SVG :D I would propose to simply whitelist them and thereby fix this issue, sounds good?

@tabdon
Copy link
Author

tabdon commented Mar 13, 2017

That sounds good to me. Will that require code change on source, or is there something I can do in the meantime? Thanks!

@cure53
Copy link
Owner

cure53 commented Mar 14, 2017

I'll do the change today!

cure53 added a commit that referenced this issue Mar 14, 2017
Added xlink:href to data-URI attributes
cure53 added a commit that referenced this issue Mar 14, 2017
Removed a test (H5SC #88) that is only relevant for ancient Opera
cure53 added a commit that referenced this issue Mar 14, 2017
Added test expectation for Edge
@cure53
Copy link
Owner

cure53 commented Mar 14, 2017

Okay, so the latest commit should behave as expected, can you gibe it a try please?

cure53 added a commit that referenced this issue Mar 14, 2017
Fixed some of the MSIE/Edge test expectations
@cure53
Copy link
Owner

cure53 commented Mar 15, 2017

The tests are green, closing this one for now. please reopen if anything is missing!

@cure53 cure53 closed this as completed Mar 15, 2017
@tabdon
Copy link
Author

tabdon commented Mar 15, 2017

Works great! Thank you very much.

@cure53
Copy link
Owner

cure53 commented Mar 15, 2017

Cool :)

@fruiz500
Copy link

I can confirm that, now in 2020, this is no longer working. Why is DOMPurify so hard on data URLs? It doesn't seem to allow any.

@cure53
Copy link
Owner

cure53 commented Apr 23, 2020

Because up until 2019, several browsers did not interact securely with those. If there is cases we block where you would say they should be allowed, we can always look into it.

@fruiz500
Copy link

I have a problem right now, and an impasse with Mozilla because they allow only DOMPurify as a sanitizer. Look at issue #430

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants