From 45a5c10fed3d9aa141594c80afa06d748fa0967d Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 9 Jun 2022 11:13:33 -0400 Subject: [PATCH] fix: modify safelist option if it contains both `select` and `style` Addresses CVE-2022-32209 --- lib/rails/html/sanitizer.rb | 19 ++++++++++++++++++- test/sanitizer_test.rb | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb index 5633ca1..13fb963 100644 --- a/lib/rails/html/sanitizer.rb +++ b/lib/rails/html/sanitizer.rb @@ -141,8 +141,25 @@ 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) - options[:tags] || self.class.allowed_tags + if options[:tags] + remove_safelist_tag_combinations(options[:tags]) + else + self.class.allowed_tags + end end def allowed_attributes(options) diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index 5bf188e..592ed7e 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -581,6 +581,25 @@ 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) + end + refute_includes(sanitized, "style") + end + protected def xpath_sanitize(input, options = {}) @@ -641,4 +660,8 @@ def convert_to_css_hex(string, escape_parens=false) def libxml_2_9_14_recovery? 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