Skip to content

Commit

Permalink
fix: modify safelist option if it contains both select and style
Browse files Browse the repository at this point in the history
Addresses CVE-2022-32209
  • Loading branch information
flavorjones committed Jun 9, 2022
1 parent 045774a commit 45a5c10
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
19 changes: 18 additions & 1 deletion lib/rails/html/sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions test/sanitizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,25 @@ 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)
end
refute_includes(sanitized, "style")
end

protected

def xpath_sanitize(input, options = {})
Expand Down Expand Up @@ -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

0 comments on commit 45a5c10

Please sign in to comment.