From f3ba1a839a35f2ba7f941c15e239a1cb379d56ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 21 Mar 2018 15:23:56 -0400 Subject: [PATCH] Make sure we address CVE-2018-8048 Even that the issue was fixed on loofah we have our own logic to scrub attributes so when the whitelist serializer is used the issue was still present. Fix CVE-2018-3741. --- lib/rails/html/scrubbers.rb | 2 ++ rails-html-sanitizer.gemspec | 2 +- test/sanitizer_test.rb | 36 ++++++++++++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb index 57c4034..42436f4 100644 --- a/lib/rails/html/scrubbers.rb +++ b/lib/rails/html/scrubbers.rb @@ -153,6 +153,8 @@ def scrub_attribute(node, attr_node) end node.remove_attribute(attr_node.name) if attr_name == 'src' && attr_node.value !~ /[^[:space:]]/ + + Loofah::HTML5::Scrub.force_correct_attribute_escaping! node end end diff --git a/rails-html-sanitizer.gemspec b/rails-html-sanitizer.gemspec index 698ff4b..e774244 100644 --- a/rails-html-sanitizer.gemspec +++ b/rails-html-sanitizer.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |spec| spec.test_files = Dir["test/**/*"] spec.require_paths = ["lib"] - spec.add_dependency "loofah", "~> 2.0" + spec.add_dependency "loofah", "~> 2.2", ">= 2.2.2" spec.add_development_dependency "bundler", "~> 1.3" spec.add_development_dependency "rake" diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb index 9651c94..7bcab6f 100644 --- a/test/sanitizer_test.rb +++ b/test/sanitizer_test.rb @@ -385,13 +385,13 @@ def test_should_sanitize_attributes def test_should_sanitize_illegal_style_properties raw = %(display:block; position:absolute; left:0; top:0; width:100%; height:100%; z-index:1; background-color:black; background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg); background-x:center; background-y:center; background-repeat:repeat;) - expected = %(display: block; width: 100%; height: 100%; background-color: black; background-x: center; background-y: center;) + expected = %(display:block;width:100%;height:100%;background-color:black;background-x:center;background-y:center;) assert_equal expected, sanitize_css(raw) end def test_should_sanitize_with_trailing_space raw = "display:block; " - expected = "display: block;" + expected = "display:block;" assert_equal expected, sanitize_css(raw) end @@ -484,6 +484,38 @@ def test_allow_data_attribute_if_requested assert_equal %(foo), white_list_sanitize(text, attributes: ['data-foo']) end + def test_uri_escaping_of_href_attr_in_a_tag_in_white_list_sanitizer + html = %{test} + + text = white_list_sanitize(html) + + assert_equal %{test}, text + end + + def test_uri_escaping_of_src_attr_in_a_tag_in_white_list_sanitizer + html = %{test} + + text = white_list_sanitize(html) + + assert_equal %{test}, text + end + + def test_uri_escaping_of_name_attr_in_a_tag_in_white_list_sanitizer + html = %{test} + + text = white_list_sanitize(html) + + assert_equal %{test}, text + end + + def test_uri_escaping_of_name_action_in_a_tag_in_white_list_sanitizer + html = %{test} + + text = white_list_sanitize(html, attributes: ['action']) + + assert_equal %{test}, text + end + protected def xpath_sanitize(input, options = {})