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

The marked-element in insecure by default #40

Open
therealmik opened this issue Oct 25, 2016 · 3 comments
Open

The marked-element in insecure by default #40

therealmik opened this issue Oct 25, 2016 · 3 comments

Comments

@therealmik
Copy link

Description

The marked-element has sanitize set to false by default, meaning that users of it are vulnerable to XSS unless it occurs to them to add the word sanitize.

It would be more sensible to add a noSanitize property (that defaults to false), and people that really trust their markdown can set it.

It will be obvious to people who need no-sanitize that something's wrong, and most likely no harm will come to them as a result, unlike the current situation.

Expected outcome

That Polymer projects are not vulnerable to XSS

Actual outcome

Users of the marked-element need to ensure they pass the sanitize attribute to their tags, or they'll be vulnerable to XSS when using user-generated markdown.

@stramel
Copy link
Collaborator

stramel commented Apr 1, 2017

These properties are driven by the marked library API. I would bring it up over there. I don't know enough about the details about it.

@therealmik
Copy link
Author

I'm not proposing an upstream change, just proposing that we enable secure markdown by default, and allow people that want allow arbitrary html (including scripts) to pass a no-sanitize for rendering trusted markdown.

This poor default has caused numerous XSS vulnerabilities.

So the proposal is simply to remove the sanitize boolean property, and create a noSanitize property that has the inverse meaning.

@sebs
Copy link
Contributor

sebs commented May 29, 2017

This is a property of the used markdown lib? Sounds like a sensible default. ;) If there is agreement we should take this in, I can make up a PR for it. Thanks for the indepth report btw

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

No branches or pull requests

4 participants