Sanitizing hangs on embedded SVG data in CSS style attribute #90
Comments
|
I have the same hang with different attributes: require 'loofah'
str = "cursor: pointer !important; display: block !important; white-space: nowrap !important; float: left !important; margin-left: 1px !important; vertical-align: top !important; overflow: hidden !important; height: 18px !important; padding: 0px 4px 0px 23px !important; border-width: 1px 1px 1px 0px !important; border-top-style: solid !important; border-right-style: solid !important; border-bottom-style: solid !important; border-top-color: rgb(226, 226, 226) !important; border-right-color: rgb(191, 191, 191) !important; border-bottom-color: rgb(185, 185, 185) !important; text-shadow: rgb(255, 255, 255) -1px 1px 0px !important; line-height: 20px !important; border-top-left-radius: 0px !important; border-bottom-left-radius: 0px !important; border-top-right-radius: 2px !important; border-bottom-right-radius: 2px !important; background-color: rgb(236, 236, 236) !important; background-image: -webkit-linear-gradient(top, rgb(254, 254, 254) 0%, rgb(236, 236, 236) 100%) !important;"
Loofah::HTML5::Scrub.scrub_css strruby 2.2.0 |
|
Using a css parser like https://github.com/rgrove/crass vs regex could also help avoiding such issues. See also html5lib/html5lib-python#152 |
|
@mfazekas Apart from the actual implementation, there are some trade-offs when switching to crass: That said, if we can come up with a patch to update scrub_css to use crass, it might be worth testing! |
|
Looking. |
|
@mfazekas I cannot reproduce with your string, using nokogiri 1.6.6.2 and ruby 2.2.2p95. @matthewtidd I can reproduce with your string, same config, where there's a delay of ~3s on my laptop. |
|
I agree with @mfazekas and html5lib/html5lib-python#152 that we shouldn't be using regexes to parse CSS. It's stupid, and it's only here because Loofah's heritage is from html5lib. I'm not sure what to do here. I'd rather break CSS parsing than allow for a DOS vector, which makes me want to revert 37e5011 and related commits. Thoughts? |
|
I think that reverting is the safer patch. Later we can think in changing strategy to use a CSS parser. |
|
I don't have any specific opinion on this 🎀 |
|
I'm with @rafaelfranca on this 👍 |
|
OK, proposing this release schedule:
presuming I can get a CSS parser to work. I'll start with |
at the cost of breaking support for negative CSS values. Related to #90.
|
Sounds good to me, thanks Mike 👍
Kasper |
|
Just a note that v2.1.0.rc1 has been released which re-implements this logic on top of Crass. Please follow or comment on #91. Thanks, all! |
|
Thank you @flavorjones!! |
That block of html causes the scrub!(:strip) function to hang. The offending line of code is:
lib/loofah/html5/scrub.rb:70
The regex itself runs fine on that style tag, but running that regex after the gsub call on line 67 causes it to hang. I would try to fix it myself, but I'm not fluent enough with regex to be able to do that. This appears to be a bug with html5lib as well (where this code was copied) so I can post an issue on their bug tracker if that is preferable.
I'm going to work on a custom scrubber to remove all url attributes completely for now but some advice would be much appreciated.
The text was updated successfully, but these errors were encountered: