rails / rails-html-sanitizer

MIT License
302 stars 80 forks source link

Should 'class' be removed from the default safe list for attributes? #168

Closed moritzhoeppner closed 1 year ago

moritzhoeppner commented 1 year ago

Hi there,

I read your explanation in https://github.com/rails/rails-html-sanitizer/issues/104 and understand why 'style' should not be on the safe list for attributes. But it seems to me that some of the concerns about 'style' also apply to 'class' (although certainly not all of them).

If I'm getting this right, it depends on the CSS of my application whether 'class' is safe. Consider, for example, a class 'overlay' that simply covers the whole page and the following input: <div class="overlay">[Phishing site]</div>

Probably such an overlay would be at least transparent or even invisible (let's say with "display: none") until made visible via JavaScript. But still, somewhere in the application there is likely a class with, say, "background-color: white" and another class with "display: block". So the input could be: <div class="overlay [class that sets background-color] [class that sets display]">[Phishing site]</div>

Especially when the application uses some kind of atomic CSS approach, the class attribute seems dangerous.

So, as far as I can see it, it would be best to remove 'class' from the default safe list. Or am I getting something wrong here?

rafaelfranca commented 1 year ago

style is mostly disabled because you can use url CSS attribute to make request to third-party websites, not really because you can hide or show some elements. You can't do that with classes, so no, it should not be removed from the safe list of attributes.