Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scrub_css drops allowed css functions from shorthand css properties #199

Closed
Iwaide opened this issue Jan 13, 2021 · 3 comments
Closed

scrub_css drops allowed css functions from shorthand css properties #199

Iwaide opened this issue Jan 13, 2021 · 3 comments

Comments

@Iwaide
Copy link

Iwaide commented Jan 13, 2021

Using CSS functions for the values of shorthand CSS properties will cause them to be removed, even if they are whitelisted, such as linear-gradient

[1] pry(main)> puts Loofah::fragment('<h1 style="background: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>').scrub!(:prune)
<h1 style="background:#ffff66;">here</h1>

I've figured out that I need to fix the following lines, but I don't know how to fix it.

elsif SafeList::SHORTHAND_CSS_PROPERTIES.include?(name.split("-").first)
value = node[:value].split.map do |keyword|
if SafeList::ALLOWED_CSS_KEYWORDS.include?(keyword) || keyword =~ CSS_KEYWORDISH
keyword
end
end.compact

How can I avoid this?
Or could you please tell me how to fix it?

@flavorjones
Copy link
Owner

Hi, @Iwaide. Thanks for opening this issue, and sorry you're having problems.

This looks like it's a bug whenever a CSS function (linear-gradient() in your example) appears in any of the variations of background, border, margin, or padding properties. So, for example, background-color is fine:

Loofah::fragment(%Q[<h1 style="background-color: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>]).scrub!(:prune)
# => <h1 style="background-color: linear-gradient(transparent 50%, #ffff66 50%);">here</h1>

as is color:

Loofah::fragment(%Q[<h1 style="color: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>]).scrub!(:prune)
# => <h1 style="color: linear-gradient(transparent 50%, #ffff66 50%);">here</h1>

Give me a few hours, I should be able to fix this quickly. In the meantime, you can work around it by using the full property name (e.g., background-color).

@Iwaide
Copy link
Author

Iwaide commented Jan 14, 2021

@flavorjones, thanks for the quick reply.
I was very surprised to see that a Pull Request has already been created!
I will use the full property name as you suggested until it is released.

@flavorjones
Copy link
Owner

@Iwaide Loofah v2.9.0 has been released with this fix! Thanks again for reporting it, and thanks for using Loofah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants