Skip to content

port: 1.4.4 changes #147

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

Merged
merged 7 commits into from
Dec 13, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,41 @@

*seyerian*

## 1.4.4 / 2022-12-13

* Address inefficient regular expression complexity with certain configurations of Rails::Html::Sanitizer.

Fixes CVE-2022-23517. See
[GHSA-5x79-w82f-gw8w](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-5x79-w82f-gw8w)
for more information.

*Mike Dalessio*

* Address improper sanitization of data URIs.

Fixes CVE-2022-23518 and #135. See
[GHSA-mcvf-2q2m-x72m](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-mcvf-2q2m-x72m)
for more information.

*Mike Dalessio*

* Address possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.

Fixes CVE-2022-23520. See
[GHSA-rrfc-7g8p-99q8](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-rrfc-7g8p-99q8)
for more information.

*Mike Dalessio*

* Address possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.

Fixes CVE-2022-23519. See
[GHSA-9h9g-93gc-623h](https://github.com/rails/rails-html-sanitizer/security/advisories/GHSA-9h9g-93gc-623h)
for more information.

*Mike Dalessio*


## 1.4.3 / 2022-06-09

* Address a possible XSS vulnerability with certain configurations of Rails::Html::Sanitizer.
Expand Down
19 changes: 1 addition & 18 deletions lib/rails/html/sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,25 +141,8 @@ 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)
if options[:tags]
remove_safelist_tag_combinations(options[:tags])
else
self.class.allowed_tags
end
options[:tags] || self.class.allowed_tags
end

def allowed_attributes(options)
Expand Down
2 changes: 1 addition & 1 deletion lib/rails/html/sanitizer/version.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Rails
module Html
class Sanitizer
VERSION = "1.4.2"
VERSION = "1.5.0.dev"
end
end
end
16 changes: 7 additions & 9 deletions lib/rails/html/scrubbers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def attributes=(attributes)
end

def scrub(node)
if node.cdata?
text = node.document.create_text_node node.text
node.replace text
if Loofah::HTML5::Scrub.cdata_needs_escaping?(node)
replacement = Loofah::HTML5::Scrub.cdata_escape(node)
node.replace(replacement)
return CONTINUE
end
return CONTINUE if skip_node?(node)
Expand Down Expand Up @@ -140,15 +140,13 @@ def scrub_attribute(node, attr_node)
end

if Loofah::HTML5::SafeList::ATTR_VAL_IS_URI.include?(attr_name)
# this block lifted nearly verbatim from HTML5 sanitization
val_unescaped = CGI.unescapeHTML(attr_node.value).gsub(Loofah::HTML5::Scrub::CONTROL_CHARACTERS,'').downcase
if val_unescaped =~ /^[a-z0-9][-+.a-z0-9]*:/ && ! Loofah::HTML5::SafeList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(Loofah::HTML5::SafeList::PROTOCOL_SEPARATOR)[0])
attr_node.remove
end
return if Loofah::HTML5::Scrub.scrub_uri_attribute(attr_node)
end

if Loofah::HTML5::SafeList::SVG_ATTR_VAL_ALLOWS_REF.include?(attr_name)
attr_node.value = attr_node.value.gsub(/url\s*\(\s*[^#\s][^)]+?\)/m, ' ') if attr_node.value
Loofah::HTML5::Scrub.scrub_attribute_that_allows_local_ref(attr_node)
end

if Loofah::HTML5::SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == 'xlink:href' && attr_node.value =~ /^\s*[^#\s].*/m
attr_node.remove
end
Expand Down
2 changes: 1 addition & 1 deletion rails-html-sanitizer.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Gem::Specification.new do |spec|

# NOTE: There's no need to update this dependency for Loofah CVEs
# in minor releases when users can simply run `bundle update loofah`.
spec.add_dependency "loofah", "~> 2.3"
spec.add_dependency "loofah", "~> 2.19", ">= 2.19.1"

spec.add_development_dependency "bundler", ">= 1.3"
spec.add_development_dependency "rake"
Expand Down
137 changes: 117 additions & 20 deletions test/sanitizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -587,23 +587,124 @@ 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)
%w[text/plain text/css image/png image/gif image/jpeg].each do |mediatype|
define_method "test_mediatype_#{mediatype}_allowed" do
input = %Q(<img src="data:#{mediatype};base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = input
actual = safe_list_sanitize(input)
assert_equal(expected, actual)

input = %Q(<img src="DATA:#{mediatype};base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = input
actual = safe_list_sanitize(input)
assert_equal(expected, actual)
end
refute_includes(sanitized, "style")
end

def test_mediatype_text_html_disallowed
input = %q(<img src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = %q(<img>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)

input = %q(<img src="DATA:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = %q(<img>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)
end

def test_mediatype_image_svg_xml_disallowed
input = %q(<img src="data:image/svg+xml;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = %q(<img>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)

input = %q(<img src="DATA:image/svg+xml;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">)
expected = %q(<img>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)
end

def test_mediatype_other_disallowed
input = %q(<a href="data:foo;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">foo</a>)
expected = %q(<a>foo</a>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)

input = %q(<a href="DATA:foo;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4=">foo</a>)
expected = %q(<a>foo</a>)
actual = safe_list_sanitize(input)
assert_equal(expected, actual)
end

def test_scrubbing_svg_attr_values_that_allow_ref
input = %Q(<div fill="yellow url(http://bad.com/) #fff">hey</div>)
expected = %Q(<div fill="yellow #fff">hey</div>)
actual = scope_allowed_attributes %w(fill) do
safe_list_sanitize(input)
end

assert_equal(expected, actual)
end

def test_style_with_css_payload
input, tags = "<style>div > span { background: \"red\"; }</style>", ["style"]
expected = "<style>div &gt; span { background: \"red\"; }</style>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

def test_combination_of_select_and_style_with_css_payload
input, tags = "<select><style>div > span { background: \"red\"; }</style></select>", ["select", "style"]
expected = "<select><style>div &gt; span { background: \"red\"; }</style></select>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

def test_combination_of_select_and_style_with_script_payload
input, tags = "<select><style><script>alert(1)</script></style></select>", ["select", "style"]
expected = "<select><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></select>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

def test_combination_of_svg_and_style_with_script_payload
input, tags = "<svg><style><script>alert(1)</script></style></svg>", ["svg", "style"]
expected = "<svg><style>&lt;script&gt;alert(1)&lt;/script&gt;</style></svg>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

def test_combination_of_math_and_style_with_img_payload
input, tags = "<math><style><img src=x onerror=alert(1)></style></math>", ["math", "style"]
expected = "<math><style>&lt;img src=x onerror=alert(1)&gt;</style></math>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)

input, tags = "<math><style><img src=x onerror=alert(1)></style></math>", ["math", "style", "img"]
expected = "<math><style>&lt;img src=x onerror=alert(1)&gt;</style></math>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

def test_combination_of_svg_and_style_with_img_payload
input, tags = "<svg><style><img src=x onerror=alert(1)></style></svg>", ["svg", "style"]
expected = "<svg><style>&lt;img src=x onerror=alert(1)&gt;</style></svg>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)

input, tags = "<svg><style><img src=x onerror=alert(1)></style></svg>", ["svg", "style", "img"]
expected = "<svg><style>&lt;img src=x onerror=alert(1)&gt;</style></svg>"
actual = safe_list_sanitize(input, tags: tags)

assert_equal(expected, actual)
end

protected
Expand Down Expand Up @@ -673,8 +774,4 @@ def libxml_2_9_14_recovery_lt_bang?
# then reverted in 2.10.0, see https://gitlab.gnome.org/GNOME/libxml2/-/issues/380
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