Incomplete fix for CVE-2022-32209 (XSS in Rails::Html::Sanitizer under certain configurations)

While building a PoC for CVE-2022-32209, I noticed that I could not fix my vulnerable application by updating https://github.com/rails/rails-html-sanitizer from 1.4.2 to 1.4.3 even though the Hackerone report about this vulnerability suggested that this should fix it (see here: https://hackerone.com/reports/1530898). I built this app with Rails 7.0.3.1 by just running "rails new", adding `config.action_view.sanitized_allowed_tags = ["select", "style"]` to the file `config/application.rb` and creating an endpoint that reflected a parameter after sanitizing it (ERB: `<p>Hello <%= sanitize @name %></p>`). When using the payload `<select><style><script>alert("XSS")</script></style></select>` for the parameter I got an alert no matter what the version of rails-html-sanitizer was. I believe the reason is the following. There are two ways you can pass the list of allowed tags to the sanitizer. One is via a list of tags stored in a class attribute, the other is via an argument passed into the `sanitize` method. The fix only considered the second way but the first one was forgotten. See the commit with the fix here: https://github.com/rails/rails-html-sanitizer/commit/c871aa4034d3d80ad67cf33a3462154b0a0fb477#diff-0daf33b9062eb5ccdeae86ed8bf2662a6e8669f1a7db590802b7f3b36ea47426R159 The relevant part of the code is this: ```ruby module Rails module Html class SafeListSanitizer < Sanitizer ... 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 end ... end end end ``` Method `remove_safelist_tag_combinations` is introduced to remove `style` from the allow list if `select` is in there. However, within method `allowed_tags` this cleanup is only applied to the tag list in the `options`, not to ` self.class.allowed_tags`, the list stored in the sanitizer class. However, it seems that the configuration in `config/application.rb` which I've set above put the list into the class variable (I've sprinkled a few `puts` here and there to confirm that). Moreover, when moving the allow list from `config/application.rb` into the ERB template (`<p>Hello <%= sanitize @name, tags: ["select", "style"] %></p>`), the update from 1.4.2 to 1.4.3 does fix the problem. The following patch to https://github.com/rails/rails-html-sanitizer should complete the fix for CVE-2022-32209 (also added a test, which is mostly copy&paste from the test method `test_disallow_the_dangerous_safelist_combination_of_select_and_style` found in the commit linked above): ``` From fb9882599684f5796805107ec98f6a18bba722ec Mon Sep 17 00:00:00 2001 From: Dominic Breuker <[email protected]> Date: Fri, 29 Jul 2022 23:08:57 +0200 Subject: [PATCH] disallow select and style in safelist also when specified in sanitizer class allowed_tags list --- lib/rails/html/sanitizer.rb | 6 +----- test/sanitizer_test.rb | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb index 97503c8..4a0d43f 100644 --- a/lib/rails/html/sanitizer.rb +++ b/lib/rails/html/sanitizer.rb @@ -155,11 +155,7 @@ module Rails end def allowed_tags(options) - if options[:tags] - remove_safelist_tag_combinations(options[:tags]) - else - self.class.allowed_tags - end + remove_safelist_tag_combinations(options[:tags] || self.class.allowed_tags) end def allowed_attributes(options) diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index e3ce218..3f0a88a 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -606,6 +606,26 @@ class SanitizersTest < Minitest::Test refute_includes(sanitized, "style") end + def test_disallow_the_dangerous_safelist_combination_of_select_and_style_in_class_tag_list + scope_allowed_tags(["select", "style"]) do + input = "<select><style><script>alert(1)</script></style></select>" + warning = /WARNING: Rails::Html::SafeListSanitizer: removing 'style' from safelist/ + sanitized = nil + invocation = Proc.new { sanitized = safe_list_sanitize(input) } + + 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 + end + protected def xpath_sanitize(input, options = {}) -- 2.35.1 ``` ## Impact It is possible to bypass Rails::Html::SafeListSanitizer filtering and perform an XSS attack.