rails/rails-html-sanitizer

iframe are scrubbed by default?

paul-mesnilgrente opened this issue · 3 comments

Raising this issue as I feel like it's a bug, or maybe a lack of documentation.

I ran a script to remove all the inline styles from the database. And it turned out that it removed all the iframes as well. Here's a summary of it

scrubber = Rails::Html::TargetScrubber.new
scrubber.attributes = ['style']

html_with_iframe = '<iframe style="display: none" width="1231" height="699" src="https://www.youtube.com/embed/abcd"></iframe>'
html_fragment = Loofah.fragment(html_with_iframe)
html_fragment.scrub!(scrubber) # scrubbed the iframe
new_html = html_fragment.to_s # empty string

To make this work, I actually have to tell the scrubber to not scrub any tags with scrubber.tags = [] even if scrubber.tags returns nil after the initialization.

Yes, it is scrubbed by default unless you define which tags you want to scrub. By default only those tags are allowed https://github.com/flavorjones/loofah/blob/07e89a5794b882c82f3d0fea39252e56948936e7/lib/loofah/html5/safelist.rb#L809.

This is not a bug, it is by design to make sure the default behavior of the scrubbers included on this library, with no configuration, is secure. Thank you for the issue.

Might be good to add some documentations on the README. Looking at the example:

scrubber = Rails::Html::TargetScrubber.new
scrubber.tags = ['img']

html_fragment = Loofah.fragment('<a><img/ ></a>')
html_fragment.scrub!(scrubber)
html_fragment.to_s # => "<a></a>"

It's really hard to see that scrubbing attributes only will also scrub some tags. The only place where I could find documentation is on

# If not, elements are stripped based on Loofahs +HTML5::Scrub.allowed_element?+.

What do you think? Happy to create a quick PR if needed.

Makes sense to improve the documentation. Feel free to open a PR