search

ifad / data-confirm-modal

Makes Rails' link_to confirm: 'foo' build a Bootstrap Modal instead of calling the browser's confirm() API.
MIT License
271 stars 115 forks source link

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

Open licatajustin opened 5 years ago

licatajustin commented 5 years ago

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 commented 5 years ago

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.

licatajustin commented 5 years ago

@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?