Merge pull request #147 from rails/flavorjones-port-1.4.4-changes

port: 1.4.4 changes

CHANGELOG.md

@@ -7,6 +7,41 @@
 
   *seyerian*
 
+## 1.4.4 / 2022-12-13
+
+* Address inefficient regular expression complexity with certain configurations of Rails::Html::Sanitizer.
+
+  Fixes CVE-2022-23517. See
+  [GHSA-5x79-w82f-gw8w](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-5x79-w82f-gw8w)
+  for more information.
+
+  *Mike Dalessio*
+
+* Address improper sanitization of data URIs.
+
+  Fixes CVE-2022-23518 and #135. See
+  [GHSA-mcvf-2q2m-x72m](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-mcvf-2q2m-x72m)
+  for more information.
+
+  *Mike Dalessio*
+
+* Address possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.
+
+  Fixes CVE-2022-23520. See
+  [GHSA-rrfc-7g8p-99q8](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-rrfc-7g8p-99q8)
+  for more information.
+
+  *Mike Dalessio*
+
+* Address possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.
+
+  Fixes CVE-2022-23519. See
+  [GHSA-9h9g-93gc-623h](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-9h9g-93gc-623h)
+  for more information.
+
+  *Mike Dalessio*
+
+
 ## 1.4.3 / 2022-06-09
 
 * Address a possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.

lib/rails/html/sanitizer.rb

@@ -141,25 +141,8 @@ def sanitize_css(style_string)
 
       private
 
-      def loofah_using_html5?
-        # future-proofing, see https://github.com/flavorjones/loofah/pull/239
-        Loofah.respond_to?(:html5_mode?) && Loofah.html5_mode?
-      end
-
-      def remove_safelist_tag_combinations(tags)
-        if !loofah_using_html5? && tags.include?("select") && tags.include?("style")
-          warn("WARNING: #{self.class}: removing 'style' from safelist, should not be combined with 'select'")
-          tags.delete("style")
-        end
-        tags
-      end
-
       def allowed_tags(options)
-        if options[:tags]
-          remove_safelist_tag_combinations(options[:tags])
-        else
-          self.class.allowed_tags
-        end
+        options[:tags] || self.class.allowed_tags
       end
 
       def allowed_attributes(options)

lib/rails/html/sanitizer/version.rb

@@ -1,7 +1,7 @@
 module Rails
   module Html
     class Sanitizer
-      VERSION = "1.4.2"
+      VERSION = "1.5.0.dev"
     end
   end
 end

lib/rails/html/scrubbers.rb

@@ -62,9 +62,9 @@ def attributes=(attributes)
       end
 
       def scrub(node)
-        if node.cdata?
-          text = node.document.create_text_node node.text
-          node.replace text
+        if Loofah::HTML5::Scrub.cdata_needs_escaping?(node)
+          replacement = Loofah::HTML5::Scrub.cdata_escape(node)
+          node.replace(replacement)
           return CONTINUE
         end
         return CONTINUE if skip_node?(node)
@@ -140,15 +140,13 @@ def scrub_attribute(node, attr_node)
                     end
 
         if Loofah::HTML5::SafeList::ATTR_VAL_IS_URI.include?(attr_name)
-          # this block lifted nearly verbatim from HTML5 sanitization
-          val_unescaped = CGI.unescapeHTML(attr_node.value).gsub(Loofah::HTML5::Scrub::CONTROL_CHARACTERS,'').downcase
-          if val_unescaped =~ /^[a-z0-9][-+.a-z0-9]*:/ && ! Loofah::HTML5::SafeList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(Loofah::HTML5::SafeList::PROTOCOL_SEPARATOR)[0])
-            attr_node.remove
-          end
+          return if Loofah::HTML5::Scrub.scrub_uri_attribute(attr_node)
         end
+
         if Loofah::HTML5::SafeList::SVG_ATTR_VAL_ALLOWS_REF.include?(attr_name)
-          attr_node.value = attr_node.value.gsub(/url\s*\(\s*[^#\s][^)]+?\)/m, ' ') if attr_node.value
+          Loofah::HTML5::Scrub.scrub_attribute_that_allows_local_ref(attr_node)
         end
+
         if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == 'xlink:href' && attr_node.value =~ /^\s*[^#\s].*/m
           attr_node.remove
         end

rails-html-sanitizer.gemspec

@@ -26,7 +26,7 @@ Gem::Specification.new do |spec|
 
   # NOTE: There's no need to update this dependency for Loofah CVEs
   # in minor releases when users can simply run `bundle update loofah`.
-  spec.add_dependency "loofah", "~> 2.3"
+  spec.add_dependency "loofah", "~> 2.19", ">= 2.19.1"
 
   spec.add_development_dependency "bundler", ">= 1.3"
   spec.add_development_dependency "rake"

test/sanitizer_test.rb

@@ -587,23 +587,124 @@ def test_exclude_node_type_comment
     assert_equal("<div>text</div><b>text</b>", safe_list_sanitize("<div>text</div><!-- comment --><b>text</b>"))
   end
 
-  def test_disallow_the_dangerous_safelist_combination_of_select_and_style
-    input = "<select><style><script>alert(1)</script></style></select>"
-    tags = ["select", "style"]
-    warning = /WARNING: Rails::Html::SafeListSanitizer: removing 'style' from safelist/
-    sanitized = nil
-    invocation = Proc.new { sanitized = safe_list_sanitize(input, tags: tags) }
-
-    if html5_mode?
-      # if Loofah is using an HTML5 parser,
-      #   then "style" should be removed by the parser as an invalid child of "select"
-      assert_silent(&invocation)
-    else
-      # if Loofah is using an HTML4 parser,
-      #   then SafeListSanitizer should remove "style" from the safelist
-      assert_output(nil, warning, &invocation)
+  %w[text/plain text/css image/png image/gif image/jpeg].each do |mediatype|
+    define_method "test_mediatype_#{mediatype}_allowed" do
+      input = %Q(<img src="data:#{mediatype};base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+      expected = input
+      actual = safe_list_sanitize(input)
+      assert_equal(expected, actual)
+
+      input = %Q(<img src="DATA:#{mediatype};base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+      expected = input
+      actual = safe_list_sanitize(input)
+      assert_equal(expected, actual)
     end
-    refute_includes(sanitized, "style")
+  end
+
+  def test_mediatype_text_html_disallowed
+    input = %q(<img src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+    expected = %q(<img>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+
+    input = %q(<img src="DATA:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+    expected = %q(<img>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+  end
+
+  def test_mediatype_image_svg_xml_disallowed
+    input = %q(<img src="data:image/svg+xml;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+    expected = %q(<img>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+
+    input = %q(<img src="DATA:image/svg+xml;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
+    expected = %q(<img>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+  end
+
+  def test_mediatype_other_disallowed
+    input = %q(<a href="data:foo;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">foo</a>)
+    expected = %q(<a>foo</a>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+
+    input = %q(<a href="DATA:foo;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">foo</a>)
+    expected = %q(<a>foo</a>)
+    actual = safe_list_sanitize(input)
+    assert_equal(expected, actual)
+  end
+
+  def test_scrubbing_svg_attr_values_that_allow_ref
+    input = %Q(<div fill="yellow url(http://bad.com/) #fff">hey</div>)
+    expected = %Q(<div fill="yellow #fff">hey</div>)
+    actual = scope_allowed_attributes %w(fill) do
+      safe_list_sanitize(input)
+    end
+
+    assert_equal(expected, actual)
+  end
+
+  def test_style_with_css_payload
+    input, tags = "<style>div > span { background: \"red\"; }</style>", ["style"]
+    expected = "<style>div &gt; span { background: \"red\"; }</style>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+  end
+
+  def test_combination_of_select_and_style_with_css_payload
+    input, tags = "<select><style>div > span { background: \"red\"; }</style></select>", ["select", "style"]
+    expected = "<select><style>div &gt; span { background: \"red\"; }</style></select>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+  end
+
+  def test_combination_of_select_and_style_with_script_payload
+    input, tags = "<select><style><script>alert(1)</script></style></select>", ["select", "style"]
+    expected = "<select><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></select>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+  end
+
+  def test_combination_of_svg_and_style_with_script_payload
+    input, tags = "<svg><style><script>alert(1)</script></style></svg>", ["svg", "style"]
+    expected = "<svg><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></svg>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+  end
+
+  def test_combination_of_math_and_style_with_img_payload
+    input, tags = "<math><style><img src=x onerror=alert(1)></style></math>", ["math", "style"]
+    expected = "<math><style>&lt;img src=x onerror=alert(1)&gt;</style></math>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+
+    input, tags = "<math><style><img src=x onerror=alert(1)></style></math>", ["math", "style", "img"]
+    expected = "<math><style>&lt;img src=x onerror=alert(1)&gt;</style></math>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+  end
+
+  def test_combination_of_svg_and_style_with_img_payload
+    input, tags = "<svg><style><img src=x onerror=alert(1)></style></svg>", ["svg", "style"]
+    expected = "<svg><style>&lt;img src=x onerror=alert(1)&gt;</style></svg>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
+
+    input, tags = "<svg><style><img src=x onerror=alert(1)></style></svg>", ["svg", "style", "img"]
+    expected = "<svg><style>&lt;img src=x onerror=alert(1)&gt;</style></svg>"
+    actual = safe_list_sanitize(input, tags: tags)
+
+    assert_equal(expected, actual)
   end
 
 protected
@@ -673,8 +774,4 @@ def libxml_2_9_14_recovery_lt_bang?
     # then reverted in 2.10.0, see https://gitlab.gnome.org/GNOME/libxml2/-/issues/380
     Nokogiri.method(:uses_libxml?).arity == -1 && Nokogiri.uses_libxml?("= 2.9.14")
   end
-
-  def html5_mode?
-    ::Loofah.respond_to?(:html5_mode?) && ::Loofah.html5_mode?
-  end
 end