Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent combination of select and style tags with the HTML4 parser #137

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion lib/rails/html/sanitizer.rb
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
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