data-confirm-modal icon indicating copy to clipboard operation
data-confirm-modal copied to clipboard

XSS Vulnerability when the modal body is user-controlled

Open licatajustin opened this issue 5 years ago • 2 comments

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}?" }) %>

licatajustin avatar Jun 18 '19 20:06 licatajustin

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 avatar Jun 24 '19 08:06 vjt

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

licatajustin avatar Jul 15 '19 14:07 licatajustin