Add support for boolean and empty attributes

flavorjones · Nov 28, 2023 · 88941ce · 88941ce
1 parent cb14ea7
commit 88941ce
Show file tree

Hide file tree

Showing 4 changed files with 182 additions and 4 deletions.
diff --git a/lib/loofah/html5/safelist.rb b/lib/loofah/html5/safelist.rb
@@ -229,6 +229,154 @@ module SafeList
         "use",
       ])
 
+      ACCEPTABLE_EMPTY_ATTRIBUTES = {
+        "*" => Set.new([
+          "title",
+        ]),
+        "area" => Set.new([
+          "alt",
+        ]),
+        "audio" => Set.new([
+          "src",
+        ]),
+        "base" => Set.new([
+          "href",
+        ]),
+        "img" => Set.new([
+          "alt",
+        ]),
+        "input" => Set.new([
+          "value",
+          "placeholder",
+        ]),
+        "li" => Set.new([
+          "value",
+        ]),
+        "link" => Set.new([
+          "href",
+        ]),
+        "meter" => Set.new([
+          "value",
+        ]),
+        "option" => Set.new([
+          "value",
+        ]),
+        "progress" => Set.new([
+          "value",
+        ]),
+        "source" => Set.new([
+          "src",
+        ]),
+        "textarea" => Set.new([
+          "placeholder",
+        ]),
+        "track" => Set.new([
+          "default",
+        ]),
+      }
+
+      ACCEPTABLE_BOOLEAN_ATTRIBUTES = {
+        "*" => Set.new([
+          "hidden",
+          "contenteditable",
+          "draggable",
+          "spellcheck",
+          "translate",
+        ]),
+        "a" => Set.new([
+          "download",
+        ]),
+        "area" => Set.new([
+          "download",
+        ]),
+        "audio" => Set.new([
+          "autoplay",
+          "controls",
+          "loop",
+          "muted",
+        ]),
+        "button" => Set.new([
+          "autofocus",
+          "disabled",
+          "formnovalidate",
+        ]),
+        "details" => Set.new([
+          "open",
+        ]),
+        "fieldset" => Set.new([
+          "disabled",
+        ]),
+        "form" => Set.new([
+          "novalidate",
+        ]),
+        "iframe" => Set.new([
+          "allowfullscreen",
+          "seamless",
+        ]),
+        "img" => Set.new([
+          "alt",
+          "ismap",
+        ]),
+        "input" => Set.new([
+          "autofocus",
+          "checked",
+          "disabled",
+          "multiple",
+          "readonly",
+          "required",
+          "formnovalidate",
+        ]),
+        "ol" => Set.new([
+          "reversed",
+        ]),
+        "optgroup" => Set.new([
+          "disabled",
+        ]),
+        "option" => Set.new([
+          "disabled",
+          "selected",
+        ]),
+        "select" => Set.new([
+          "autofocus",
+          "disabled",
+          "multiple",
+          "required",
+        ]),
+        "style" => Set.new([
+          "scoped",
+        ]),
+        "table" => Set.new([
+          "sortable",
+        ]),
+        "td" => Set.new([
+          "nowrap",
+        ]),
+        "th" => Set.new([
+          "nowrap",
+        ]),
+        "textarea" => Set.new([
+          "autofocus",
+          "disabled",
+          "readonly",
+          "required",
+        ]),
+        "track" => Set.new([
+          "default",
+        ]),
+        "video" => Set.new([
+          "autoplay",
+          "controls",
+          "loop",
+          "muted",
+          "playsinline",
+        ]),
+      }
+
+      ACCEPTABLE_BOOLEAN_OR_EMPTY_ATTRIBUTES =
+        ACCEPTABLE_BOOLEAN_ATTRIBUTES.merge(ACCEPTABLE_EMPTY_ATTRIBUTES) do |_, a, b|
+          a + b
+        end
+
       ACCEPTABLE_ATTRIBUTES = Set.new([
         "abbr",
         "accept",

diff --git a/lib/loofah/html5/scrub.rb b/lib/loofah/html5/scrub.rb
@@ -32,7 +32,9 @@ def scrub_attributes(node)
               next
             end
 
-            unless SafeList::ALLOWED_ATTRIBUTES.include?(attr_name)
+            unless SafeList::ALLOWED_ATTRIBUTES.include?(attr_name) ||
+                SafeList::ACCEPTABLE_BOOLEAN_OR_EMPTY_ATTRIBUTES[node.name]&.include?(attr_node.name) ||
+                SafeList::ACCEPTABLE_BOOLEAN_OR_EMPTY_ATTRIBUTES["*"].include?(attr_node.name)
               attr_node.remove
               next
             end
@@ -56,9 +58,11 @@ def scrub_attributes(node)
           scrub_css_attribute(node)
 
           node.attribute_nodes.each do |attr_node|
-            if attr_node.value !~ /[^[:space:]]/ && attr_node.name !~ DATA_ATTRIBUTE_NAME
-              node.remove_attribute(attr_node.name)
-            end
+            next if attr_node.value =~ /[^[:space:]]/ || attr_node.name =~ DATA_ATTRIBUTE_NAME ||
+              SafeList::ACCEPTABLE_BOOLEAN_OR_EMPTY_ATTRIBUTES[node.name]&.include?(attr_node.name) ||
+              SafeList::ACCEPTABLE_BOOLEAN_OR_EMPTY_ATTRIBUTES["*"].include?(attr_node.name)
+
+            node.remove_attribute(attr_node.name)
           end
 
           force_correct_attribute_escaping!(node)

diff --git a/test/assets/testdata_sanitizer_tests1.dat b/test/assets/testdata_sanitizer_tests1.dat
@@ -526,5 +526,23 @@
     "name": "attributes_with_embedded_quotes_II",
     "input": "<img src=notthere.jpg\"\"onerror=\"alert(2) />",
     "libxml": "<img src='notthere.jpg%22%22onerror=%22alert(2)'>"
+  },
+
+  {
+    "name": "empty_attributes_1",
+    "input": "<option value=\"\">Empty Value<script>alert(1)</script></option>",
+    "libxml": "<option value=''>Empty Value&lt;script&gt;alert(1)&lt;/script&gt;</option>"
+  },
+
+  {
+    "name": "empty_attributes_2",
+    "input": "<option value=\"\">Empty Value</script></option>",
+    "libxml": "<option value=''>Empty Value</option>"
+  },
+
+  {
+    "name": "empty_attributes_3",
+    "input": "<option value=\"\" download=\"foo-bar\">Empty Value</option>",
+    "libxml": "<option value=''>Empty Value</option>"
   }
 ]
diff --git a/test/html5/test_sanitizer.rb b/test/html5/test_sanitizer.rb
@@ -200,6 +200,14 @@ def test_should_allow_contenteditable
     check_sanitization(input, output)
   end
 
+  def test_boolean_attributes
+    input = "<video controls download></video>"
+    expected_html5 = "<video controls=''></video>"
+    output_html5 = sanitize_html5(input).tr('"', "'")
+
+    assert_equal(expected_html5, output_html5)
+  end
+
   ##
   ##  libxml2 downcases attributes, so this is moot.
   ##