Closed bkuker closed 7 years ago
cure53
commented
7 years ago Heya, I had a closer look. I think the behavior here comes from how Chrome handles the tags.
DOMPurify basically takes the DOM the browser produces - and we don't move tags around. We basically take the data, throw it into a DOM and then work on that. So, I would guess that this is a parser quirk in Chrome?
bkuker
commented
7 years ago Huh, I would not have believed it if it were not for the second "working" example I gave where all I did was change the "foreignObject" tags name to "foobar"... All three of the example files do render properly when I do not purify them, so at the very least there is some interaction. To be honest I did not test this on other browsers.
I've worked around the problem by doing a string replace of "<foreignObject" to "<removeMe" (a better name than foobar for the next guy to support the code) before sending the string through DOMPuriy, and letting it remove the element correctly.
If I run with using DOMPurify for the job I'll take a longer look at this "bug" in case there is something more interesting to it.
cure53
commented
7 years ago I found the problem: It's indeed the self-closing circle as shown in your examples. Check the output from your original example on this website to see what browsers create from that SVG markup:
https://html5sec.org/innerhtml/
INPUT Example One (original one):
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 300 100">
<g><circle cx="50" cy="50" r="40" stroke-width="3" fill="red" stroke="black" /></g>
<g>
<switch>
<foreignObject height="1" width="1" requiredExtensions="http://schemas.microsoft.com/visio/2003/SVGExtensions/">
<circle cx="150" cy="25" r="40" stroke-width="3" fill="yellow" stroke="black"/>
</foreignObject>
<g><circle cx="150" cy="75" r="40" stroke-width="3" fill="blue" stroke="black" /></g>
</switch>
</g>
<g><circle cx="250" cy="50" r="40" stroke-width="3" fill="green" stroke="black" /></g>
</svg>OUTPUT Example One (original one):
<!--?xml version="1.0" encoding="UTF-8"?-->
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 300 100">
<g><circle cx="50" cy="50" r="40" stroke-width="3" fill="red" stroke="black"></circle></g>
<g>
<switch>
<foreignObject height="1" width="1" requiredExtensions="http://schemas.microsoft.com/visio/2003/SVGExtensions/">
<circle cx="150" cy="25" r="40" stroke-width="3" fill="yellow" stroke="black">
<g><circle cx="150" cy="75" r="40" stroke-width="3" fill="blue" stroke="black"></circle></g>
<g><circle cx="250" cy="50" r="40" stroke-width="3" fill="green" stroke="black"></circle></g>
</circle></foreignObject></switch></g></svg>Now, try to change this:
<circle cx="150" cy="25" r="40" stroke-width="3" fill="yellow" stroke="black"/>
to this:
<circle cx="150" cy="25" r="40" stroke-width="3" fill="yellow" stroke="black"></circle>
As you can see, it renders well and the switch element will be at the right position. The core difference might be that an SVG rendered in an HTML document behaves slightly different compared to being rendered "standalone" as image/svg+xml. And sadly, that's browser behavior and nothing DOMPurify does wrong.
bkuker
commented
7 years ago Cool, thanks! I think this is described well enough that anyone else unfortunate enough to stumble across the problem can google it and find this. From my point of view this can be closed. I guess way more important is can this misbehavior on the browser's part be used to bypass DOMPurify in any way? I doubt it but I am NOT qualified to guess either way! Thanks!
cure53
commented
7 years ago Cool :) Security-wise, things should be fine. There is countless of those quirks and crazy artifacts - but I would be very surprised if they hit us with a bypass.
Similar bypasses exist on MSIE9 and older - that is why we for example only offer fall-back support for MSIE9 while MSIE10 is okay :D
Bottom line: SVG in text/html parses differently than SVG in image/svg+xml - and maybe at some point we need an XML-fork of DOMPurify to handle all non text/html documents.
Version 0.8.4 on chrome Version 55.0.2883.87 m (64-bit)
Trying to purify SVG files exported from Microsoft Visio. I reduced the file from 40,000+ lines to less than 20 lines that show the bug.
You will find a "yellow circle" inside a "foreignObject" inside a "switch". When this document is purified the foreignObject tag is removed. That is fine with me, but the closing tag for the switch statement is misplaced in the output.
When the yellow circle tag is changed from
<circle.../>to<circle...></circle>the problem goes away.Also, and this really makes no sense to me, changing foreignObject to foobar makes the problem go away too.
Please note that the problem is NOT that the foreignObject tag is being removed, or the effect this has on the svg. The problem is that the
</switch>ends up in totally the wrong place.Breaks:
Works:
This file has one line changed from the breaks example, how yellow circle tag is closed.
Works:
This file has 2 lines changed from the breaks example, foreignObject is changed to foobar.