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

XSS Vulnerability when the modal body is user-controlled #72

Open
licatajustin opened this issue Jun 18, 2019 · 2 comments
Open

XSS Vulnerability when the modal body is user-controlled #72

licatajustin opened this issue Jun 18, 2019 · 2 comments

Comments

@licatajustin
Copy link

If the contents of the data-confirm attribute contain user-generated content, this library opens up the app to possible XSS vulnerabilities, which makes sense given the use of the html() function here.

Is it by design to allow script tags to execute if passed into the body? I understand the desire to pass in HTML that is rendered properly in the modal body, but perhaps we can work to prevent these script tags from also executing. Thoughts?

Crude Example:

<% # Assumption: user.name = "<script>alert('oops')</script>" %>

<%= link_to("Open", "#", data: { confirm: "Are you sure you want to open this #{user.name}?" }) %>
@vjt
Copy link
Contributor

vjt commented Jun 24, 2019

Hi, many thanks for the heads up. I reviewed the approach in the forked repo, and as we all know you can't parse [X|HT]ML with regexen.

There are some interesting solutions here and here and here but I still haven't made up my mind on this, because there's onload, onerror, etc.

Any solution has its drawbacks, as this is XML and it is complex. Maybe the best solution would be a README entry saying that the body of the modal must not be user-controlled as this will lead to XSS. The real solution is to embed a full HTML sanitiser, but again I don't like that because of the corner case of having the end user provide the modal body.

@vjt vjt changed the title XSS Vulnerability XSS Vulnerability when the modal body is user-controller Jun 24, 2019
@vjt vjt changed the title XSS Vulnerability when the modal body is user-controller XSS Vulnerability when the modal body is user-controlled Jun 24, 2019
@licatajustin
Copy link
Author

@vjt, fair point -- onerror is certainly still an issue with the temporary patch I applied in the forked repo. What are your thoughts on adding a dependency like DOMPurify?

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

2 participants