Merge pull request #123 from eventfuel/enable_whitelisting_css_functions

Added ability to whitelist particular functions #122
flavorjones · Feb 6, 2018 · a414233 · a414233
2 parents f35e2d4 + 1bc5a45
commit a414233
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 1 deletion.
diff --git a/lib/loofah/html5/scrub.rb b/lib/loofah/html5/scrub.rb
@@ -79,7 +79,7 @@ def scrub_css style
           style_tree.each do |node|
             next unless node[:node] == :property
             next if node[:children].any? do |child|
-              [:url, :bad_url, :function].include? child[:node]
+              [:url, :bad_url].include?(child[:node]) || (child[:node] == :function && !WhiteList::ALLOWED_CSS_FUNCTIONS.include?(child[:name].downcase))
             end
             name = node[:name].downcase
             if WhiteList::ALLOWED_CSS_PROPERTIES.include?(name) || WhiteList::ALLOWED_SVG_PROPERTIES.include?(name)

diff --git a/lib/loofah/html5/whitelist.rb b/lib/loofah/html5/whitelist.rb
@@ -137,6 +137,8 @@ module WhiteList
       purple red right solid silver teal top transparent underline white
       yellow]
 
+      ACCEPTABLE_CSS_FUNCTIONS = Set.new %w[calc]
+
       SHORTHAND_CSS_PROPERTIES = Set.new %w[background border margin padding]
 
       ACCEPTABLE_SVG_PROPERTIES = Set.new %w[fill fill-opacity fill-rule stroke
@@ -155,6 +157,7 @@ module WhiteList
       ALLOWED_ATTRIBUTES = ACCEPTABLE_ATTRIBUTES + MATHML_ATTRIBUTES + SVG_ATTRIBUTES
       ALLOWED_CSS_PROPERTIES = ACCEPTABLE_CSS_PROPERTIES
       ALLOWED_CSS_KEYWORDS = ACCEPTABLE_CSS_KEYWORDS
+      ALLOWED_CSS_FUNCTIONS = ACCEPTABLE_CSS_FUNCTIONS
       ALLOWED_SVG_PROPERTIES = ACCEPTABLE_SVG_PROPERTIES
       ALLOWED_PROTOCOLS = ACCEPTABLE_PROTOCOLS
       ALLOWED_URI_DATA_MEDIATYPES = ACCEPTABLE_URI_DATA_MEDIATYPES

diff --git a/test/html5/test_sanitizer.rb b/test/html5/test_sanitizer.rb
@@ -275,6 +275,18 @@ def test_css_negative_value_sanitization_shorthand_css_properties
     assert_match %r/-0.05em/, sane.inner_html
   end
 
+  def test_css_function_sanitization_leaves_whitelisted_functions
+    html = "<span style=\"width:calc(5%)\">"
+    sane = Nokogiri::HTML(Loofah.scrub_fragment(html, :strip).to_html)
+    assert_match %r/calc\(5%\)/, sane.inner_html
+  end
+
+  def test_css_function_sanitization_strips_style_attributes_with_unsafe_functions
+    html = "<span style=\"width: attr(data-evil-attr)\">"
+    sane = Nokogiri::HTML(Loofah.scrub_fragment(html, :strip).to_html)
+    assert_match %r/<span><\/span>/, sane.inner_html
+  end
+
   def test_issue_90_slow_regex
     skip("timing tests are hard to make pass and have little regression-testing value")