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

there was a xss both in img and a label #721

Open
nanshihui opened this issue Nov 7, 2018 · 5 comments
Open

there was a xss both in img and a label #721

nanshihui opened this issue Nov 7, 2018 · 5 comments

Comments

@nanshihui
Copy link

nanshihui commented Nov 7, 2018

when We enter some strings,such as:
</textarea><img/id="confirm(/xss/)"/alt="/"src="/"onerror=eval(id)>'">
or you can use
[asdasd](javascript:alert&#40;&#49;&#41;)

image

The editor will execute XSS payload

When others use this editor, it is easy to get administrator rights by using XSS attack.

@atodorov
Copy link

@WesCossick do you have an ETA on this issue ? If I want to provide a fix which files/functions do I have to look at ?

@hhaidar
Copy link

hhaidar commented Nov 16, 2018

#243 (comment)
#435 (comment)
#657 (comment)

I'm not sure there's going to be a fix for this based on the comments above (which is absolutely insane and dangerous).

Some people have gotten around this by setting sanitize: true in marked (#243 (comment)) or providing their own renderer in options.renderPreview.

@marekdedic
Copy link

I am not the author, but found this very helpful:
https://github.com/showdownjs/showdown/wiki/Markdown's-XSS-Vulnerability-(and-how-to-mitigate-it)

atodorov added a commit to kiwitcms/Kiwi that referenced this issue Nov 29, 2018
This resolves medium severity XSS vulnerability which can be
exploited when previewing malicious text in the editor.

sparksuite/simplemde-markdown-editor#721
https://snyk.io/vuln/SNYK-JS-SIMPLEMDE-72570
atodorov added a commit to kiwitcms/Kiwi that referenced this issue Nov 29, 2018
This resolves medium severity XSS vulnerability which can be
exploited when previewing malicious text in the editor.

sparksuite/simplemde-markdown-editor#721
https://snyk.io/vuln/SNYK-JS-SIMPLEMDE-72570
@gseastream
Copy link

gseastream commented Sep 9, 2020

After reading all the different issues and the helpful link above, I came up with my own solution to this problem that so far seems to do exactly what I wanted. Since the sanitize option of marked is apparently deprecated now, this uses the renderPreview option of simplemde mentioned above, and is a pretty easy fix using DOMPurify:

function setupMDE(elem=null) {
    var opt = {
        renderingConfig: {
            codeSyntaxHighlighting: true
        }
    };
    if (elem != null) {
        opt["element"] = elem;
    };

    var simplemde = new SimpleMDE(opt);

    // need to use the default rendering routine
    simplemde.options.previewRender = function(plainText) {
        return DOMPurify.sanitize(simplemde.markdown(plainText));
    };
};

I discovered that using the old SimpleMDE.prototype.markdown method doesn't always work, for example when trying to use syntax highlighting (it doesn't do the highlighting). My guess is because that routine isn't bound to your own instance of simplemde, so it doesn't see the option you set saying to highlight syntax. But we're able to just set the previewRender option after initial construction, and use the object's own bound markdown method just fine.

Let me know if anything can be improved. This could also be done async (see the simplemde docs) but DOMPurify seems to be fast enough for the scale of my app. Also note that even the marked package itself says the its output should be sanitized before display, so using DOMPurify here should be a given.

@gseastream
Copy link

I've learned that EasyMDE enables this much more easily by default. For anyone finding this, EasyMDE is a fork of this project that is still maintained as of 2020.

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

No branches or pull requests

5 participants