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

xlink:href filtering (particularly in <use>) #233

Closed
phinch opened this issue Jul 10, 2017 · 4 comments
Closed

xlink:href filtering (particularly in <use>) #233

phinch opened this issue Jul 10, 2017 · 4 comments

Comments

@phinch
Copy link

phinch commented Jul 10, 2017

When filtering SVGs, DOMPurify seems to be over-eager in eliminating certain tags - I noticed it in one of my SVG <use> tags being removed. Specifically, I defined a path in an SVG file and then referred to it using a use tag later in the document:

<defs>
    <path id="path-1" d="M11.2357619,11.3004444 ..."></path>
</defs>
...
<mask id="mask-2" sketch:name="Clip 12" fill="white">
    <use xlink:href="#path-1"></use>
</mask>

but after being sanitized, the <mask> became:

<mask id="mask-2" sketch:name="Clip 12" fill="white">

</mask>

with the <use> tag removed, and the SVG image was thus missing that element when it was loaded.

In my understanding, it should be safe to have internal references in xlink:href paths like I had above, so long as other scripts in the file are sanitized. Is there any way to improve the sanitizer to only block dangerous (e.g. cross-origin) hrefs inside <use> tags?

@phinch phinch changed the title <use> tag filtering xlink:href filtering Jul 10, 2017
@phinch phinch changed the title xlink:href filtering xlink:href filtering (particularly in <use>) Jul 10, 2017
@cure53
Copy link
Owner

cure53 commented Jul 11, 2017

Hmmm, I think blocking only cross-origin xlink:href for <use> but allowing same-origin is risky as all you need to make an XSS out of it is an Open Redirect on the affected website.

I would recommend working with a hook, that checks if the xlink:href starts with # and then permit it. Should be easily doable by allowing <use> via config and then using a hook like afterSanitizeAttributes.

Would that work for you?

@cure53
Copy link
Owner

cure53 commented Jul 11, 2017

I think this might do, no?

<!doctype html>
<html>
    <head>
        <script src="../purify.js"></script>
    </head>
    <body>
        <!-- Now let's sanitize that content -->
        <script>
            'use strict';
           
            DOMPurify.addHook('afterSanitizeAttributes', function(node){
                if(node.hasAttribute('xlink:href') && !node.getAttribute('xlink:href').match(/^#/)){
                    node.remove();
                } 
            });

            // Clean HTML string and write into our DIV
            var clean = DOMPurify.sanitize("<svg>\
                <use xlink:href='#foo' />\
                <use xlink:href='//evil' />\
            </svg>", {ADD_TAGS: ['use']});
            console.dir(clean);
        </script>
    </body>
</html>

@phinch
Copy link
Author

phinch commented Jul 11, 2017

That looks good to me, thanks! I'm still pretty unfamiliar with XSS, so I wasn't sure if it was possible to sneak some javascript in after the #, but this works great for me. Thanks!

@rodrigofariow
Copy link

rodrigofariow commented Aug 30, 2022

Glad there is a work around for this! :)

#233 (comment)

Is there a reason why this is not done by default, though? Performance related?
I feel like it's a relatively common use case to have use tags on svgs.

Thank you for your time.

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