fix: disallow 'mglyph' and 'malignmark' from safe lists

https://hackerone.com/reports/2519936

Author: flavorjones

lib/rails/html/scrubbers.rb

@@ -134,6 +134,19 @@ def validate!(var, name)
           if var && !var.is_a?(Enumerable)
             raise ArgumentError, "You should pass :#{name} as an Enumerable"
           end
+
+          if var && name == :tags
+            if var.include?("mglyph")
+              warn("WARNING: 'mglyph' tags cannot be allowed by the PermitScrubber and will be scrubbed")
+              var.delete("mglyph")
+            end
+
+            if var.include?("malignmark")
+              warn("WARNING: 'malignmark' tags cannot be allowed by the PermitScrubber and will be scrubbed")
+              var.delete("malignmark")
+            end
+          end
+
           var
         end
 

test/sanitizer_test.rb

@@ -1026,6 +1026,46 @@ def test_should_sanitize_across_newlines
       assert_equal "", sanitize_css(raw)
     end
 
+    def test_should_prune_mglyph
+      # https://hackerone.com/reports/2519936
+      input = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>"
+      tags = %w(math mtext table mglyph style)
+
+      actual = nil
+      assert_output(nil, /WARNING: 'mglyph' tags cannot be allowed by the PermitScrubber/) do
+        actual = safe_list_sanitize(input, tags: tags)
+      end
+
+      acceptable_results = [
+        # libxml2
+        "<math><mtext><table><style>&lt;img src=: onerror=alert(1)&gt;</style></table></mtext></math>",
+        # libgumbo
+        "<math><mtext><style><img src=: onerror=alert(1)></style><table></table></mtext></math>",
+      ]
+
+      assert_includes(acceptable_results, actual)
+    end
+
+    def test_should_prune_malignmark
+      # https://hackerone.com/reports/2519936
+      input = "<math><mtext><table><malignmark><style><img src=: onerror=alert(1)>"
+      tags = %w(math mtext table malignmark style)
+
+      actual = nil
+      assert_output(nil, /WARNING: 'malignmark' tags cannot be allowed by the PermitScrubber/) do
+        actual = safe_list_sanitize(input, tags: tags)
+      end
+
+      acceptable_results = [
+        # libxml2
+        "<math><mtext><table><style>&lt;img src=: onerror=alert(1)&gt;</style></table></mtext></math>",
+        # libgumbo
+        "<math><mtext><style><img src=: onerror=alert(1)></style><table></table></mtext></math>",
+      ]
+
+      assert_includes(acceptable_results, actual)
+    end
+
     protected
       def safe_list_sanitize(input, options = {})
         module_under_test::SafeListSanitizer.new.sanitize(input, options)
@@ -1075,5 +1115,37 @@ class HTML4SafeListSanitizerTest < Minitest::Test
   class HTML5SafeListSanitizerTest < Minitest::Test
     @module_under_test = Rails::HTML5
     include SafeListSanitizerTest
+
+    def test_should_not_be_vulnerable_to_mglyph_namespace_confusion
+      # https://hackerone.com/reports/2519936
+      input = "<math><mtext><table><mglyph><style><img src=: onerror=alert(1)>"
+      tags = %w(math mtext table mglyph style)
+
+      result = nil
+      assert_output(nil, /WARNING/) do
+        result = safe_list_sanitize(input, tags: tags)
+      end
+
+      browser = Nokogiri::HTML5::Document.parse(result)
+      xss = browser.at_xpath("//img/@onerror")
+
+      assert_nil(xss)
+    end
+
+    def test_should_not_be_vulnerable_to_malignmark_namespace_confusion
+      # https://hackerone.com/reports/2519936
+      input = "<math><mtext><table><malignmark><style><img src=: onerror=alert(1)>"
+      tags = %w(math mtext table malignmark style)
+
+      result = nil
+      assert_output(nil, /WARNING/) do
+        result = safe_list_sanitize(input, tags: tags)
+      end
+
+      browser = Nokogiri::HTML5::Document.parse(result)
+      xss = browser.at_xpath("//img/@onerror")
+
+      assert_nil(xss)
+    end
   end if loofah_html5_support?
 end

test/scrubbers_test.rb

@@ -121,6 +121,14 @@ def test_leaves_only_supplied_tags_and_attributes
     assert_scrubbed html, '<tag></tag><tag cooler=""></tag>'
   end
 
+  def test_does_not_allow_safelisted_mglyph
+    # https://hackerone.com/reports/2519936
+    assert_output(nil, /WARNING: 'mglyph' tags cannot be allowed by the PermitScrubber/) do
+      @scrubber.tags = ["div", "mglyph", "span"]
+    end
+    assert_equal(["div", "span"], @scrubber.tags)
+  end
+
   def test_leaves_text
     assert_scrubbed("some text")
   end