CSS Scrubber is removing the builtin extended CSS color properties in `>= v2.9.0`

rocketedaway commented 2 years ago

Expected

When scrubbing HTML which makes use of the builtin extended CSS color properties in the style property they are not removed.

Actual

Builtin extended CSS color properties included in the style property of HTML strings are being removed.

Reproduction steps

The issue looks to be introduced in v2.9.0

Install Loofa v2.8.0
Run the following command in your ruby console
- Loofah.fragment('<div style="background-color: blue;">Test</div>').scrub!(:strip).to_s
You will get the following HTML string back <div style=\"background:blue;\">Test</div>
Run the following command in your ruby console
- Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
You will get the following HTML string back <div style=\"background-color: lightblue;\">Test</div>
Install Loofa v2.9.0
- This can be any version >= 2.9.0
Run the following command in your ruby console
- Loofah.fragment('<div style="background-color: blue;">Test</div>').scrub!(:strip).to_s
You will get the following HTML string back <div style=\"background:blue;\">Test</div>
Run the following command in your ruby console
- Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
You will get the following HTML string back <div>Test</div>
- I would expect <div style="background-color: lightblue;">Test</div>

flavorjones commented 2 years ago

Hi, thanks for reporting this. For clarity, I think this is what you're saying is happening? (The code snippets above are inconsistent so I just want to make sure I understand.)

#! /usr/bin/env ruby

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "loofah", "= 2.8.0"
end

Loofah.fragment('<div style="background-color: blue;">Test</div>').scrub!(:strip).to_s
# => "<div style=\"background-color: blue;\">Test</div>"

Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
# => "<div style=\"background-color: lightblue;\">Test</div>"

#! /usr/bin/env ruby

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "loofah", "= 2.18.0"
end

Loofah.fragment('<div style="background-color: blue;">Test</div>').scrub!(:strip).to_s
# => "<div style=\"background-color:blue;\">Test</div>"

Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
# => "<div>Test</div>"

and we expect that lightblue should be an acceptable color in loofah 2.18.0 and behave the same as blue.

I'll investigate!

flavorjones commented 2 years ago

Looks like we just need to include the extended colors in Loofah::HTML5::SafeList::ACCEPTABLE_CSS_KEYWORDS.

#! /usr/bin/env ruby

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "loofah", "= 2.18.0"
end

Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
# => "<div>Test</div>"

Loofah::HTML5::SafeList::ACCEPTABLE_CSS_KEYWORDS.add("lightblue")

Loofah.fragment('<div style="background-color: lightblue;">Test</div>').scrub!(:strip).to_s
# => "<div style=\"background-color:lightblue;\">Test</div>"

So you have a workaround right now if you need it. I'll schedule some work to add those colors and make a new release.

flavorjones commented 2 years ago

See #244

rocketedaway commented 2 years ago

Thanks a bunch @flavorjones!! The quick turn around is SUPER appreciated!!!

flavorjones commented 2 years ago

v2.19.0 has been shipped! Happy hacking

flavorjones / loofah