From c871aa4034d3d80ad67cf33a3462154b0a0fb477 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 9 Jun 2022 11:13:33 -0400 Subject: [PATCH 1/2] 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 ffd6764..97503c8 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 df8e64b..e3ce218 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -587,6 +587,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 = {}) @@ -647,4 +666,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 From b28cf58e957d876bfbf6e15fd8f6bcaebba44509 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 9 Jun 2022 18:21:38 -0400 Subject: [PATCH 2/2] update CHANGELOG for v1.4.3 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4805eb8..bae991e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ *seyerian* +## 1.4.3 / 2022-06-09 + +* Address a possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer. + + Prevent the combination of `select` and `style` as allowed tags in SafeListSanitizer. + + Fixes CVE-2022-32209 + + *Mike Dalessio* + ## 1.4.2 / 2021-08-23 * Slightly improve performance.