diff --git a/CHANGELOG.md b/CHANGELOG.md index bae991e..ef81d34 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb index 97503c8..ffd6764 100644 --- a/lib/rails/html/sanitizer.rb +++ b/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) diff --git a/lib/rails/html/sanitizer/version.rb b/lib/rails/html/sanitizer/version.rb index 446c687..276c9a0 100644 --- a/lib/rails/html/sanitizer/version.rb +++ b/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 diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb index 13b6d6f..6a99763 100644 --- a/lib/rails/html/scrubbers.rb +++ b/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 diff --git a/rails-html-sanitizer.gemspec b/rails-html-sanitizer.gemspec index c9637b7..653084c 100644 --- a/rails-html-sanitizer.gemspec +++ b/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" diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index e83c54d..d59a615 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -587,23 +587,124 @@ def test_exclude_node_type_comment assert_equal("
text
text", safe_list_sanitize("
text
text")) end - def test_disallow_the_dangerous_safelist_combination_of_select_and_style - input = "" - 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() + expected = input + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + + input = %Q() + 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() + expected = %q() + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + + input = %q() + expected = %q() + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + end + + def test_mediatype_image_svg_xml_disallowed + input = %q() + expected = %q() + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + + input = %q() + expected = %q() + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + end + + def test_mediatype_other_disallowed + input = %q(foo) + expected = %q(foo) + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + + input = %q(foo) + expected = %q(foo) + actual = safe_list_sanitize(input) + assert_equal(expected, actual) + end + + def test_scrubbing_svg_attr_values_that_allow_ref + input = %Q(
hey
) + expected = %Q(
hey
) + 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"] + expected = "" + 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"] + expected = "" + 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"] + expected = "" + 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"] + expected = "" + 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"] + expected = "" + actual = safe_list_sanitize(input, tags: tags) + + assert_equal(expected, actual) + + input, tags = "", ["math", "style", "img"] + expected = "" + 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"] + expected = "" + actual = safe_list_sanitize(input, tags: tags) + + assert_equal(expected, actual) + + input, tags = "", ["svg", "style", "img"] + expected = "" + 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