flavorjones / loofah

Ruby library for HTML/XML transformation and sanitization
MIT License
934 stars 138 forks source link

Regression in 2.9.0: string css attributes scrubbing #202

Closed aert closed 3 years ago

aert commented 3 years ago

This commit https://github.com/flavorjones/loofah/commit/bf13d48b7428aa36ee9dff083d017399249b2d60 seem to scrub all css attribute of type "string", which impact font-family values that are not single word.

For example, the two examples in https://github.com/flavorjones/loofah/issues/130 don't work anymore.

flavorjones commented 3 years ago

Thanks for reporting this! I'll take a look.

flavorjones commented 3 years ago

Using this as the test script:

#!/usr/bin/env ruby

require 'loofah'

text = <<~EOF
<span style="font-size: 36px; font-family: 'AvenirNext-Regular';">This style gets stripped</span>
<span style="font-size: 36px; font-family: 'Avenir Next';">This style does not get stripped</span>
EOF

puts Loofah::VERSION
puts Loofah.fragment(text).scrub!(:strip)
puts Loofah.fragment(text).scrub!(:prune)

I git-bisected and agree that this commit introduced the issue:

bf13d48b7428aa36ee9dff083d017399249b2d60 is the first bad commit
commit bf13d48b7428aa36ee9dff083d017399249b2d60
Author: Mike Dalessio <mike.dalessio@gmail.com>
Date:   Wed Jan 13 13:44:19 2021 -0500

    fix: handle CSS functions in a CSS shorthand property

    Fixes #199.

 lib/loofah/html5/scrub.rb                 | 63 ++++++++++++++++++++-----------
 test/assets/testdata_sanitizer_tests1.dat |  8 ++--
 test/html5/test_scrub.rb                  | 10 -----
 test/html5/test_scrub_css.rb              | 32 ++++++++++++++++
 4 files changed, 76 insertions(+), 37 deletions(-)
 delete mode 100644 test/html5/test_scrub.rb
 create mode 100644 test/html5/test_scrub_css.rb
flavorjones commented 3 years ago

PR in #203

flavorjones commented 3 years ago

v2.9.1 has been released fixing this issue. Thanks again for reporting it!

aert commented 3 years ago

@flavorjones No thank you, you're the one doing the great work here : ).