From 301be1abd956859e8e34925eaebf1a852f62cd9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@gmail.com>
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.
---
 lib/rails/html/scrubbers.rb  |  2 ++
 rails-html-sanitizer.gemspec |  2 +-
 test/sanitizer_test.rb       | 46 +++++++++++++++++++++++++++++++++++++-------
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 1e6f887..3695bb2 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -152,6 +152,8 @@ module Rails
         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 a600053..cb7a5fb 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 f72b702..552603c 100644
--- a/test/sanitizer_test.rb
+++ b/test/sanitizer_test.rb
@@ -58,11 +58,11 @@ class SanitizersTest < Minitest::Test
   end
 
   def test_strip_invalid_html
-    assert_equal "", full_sanitize("<<<bad html")
+    assert_equal "&lt;&lt;", full_sanitize("<<<bad html")
   end
 
   def test_strip_nested_tags
-    expected = "Weia onclick='alert(document.cookie);'/&gt;rdos"
+    expected = "Wei&lt;a onclick='alert(document.cookie);'/&gt;rdos"
     input = "Wei<<a>a onclick='alert(document.cookie);'</a>/>rdos"
     assert_equal expected, full_sanitize(input)
   end
@@ -99,7 +99,7 @@ class SanitizersTest < Minitest::Test
   end
 
   def test_strip_tags_with_many_open_quotes
-    assert_equal "", full_sanitize("<<<bad html>")
+    assert_equal "&lt;&lt;", full_sanitize("<<<bad html>")
   end
 
   def test_strip_tags_with_sentence
@@ -123,7 +123,7 @@ class SanitizersTest < Minitest::Test
   end
 
   def test_strip_links_with_tags_in_tags
-    expected = "a href='hello'&gt;all <b>day</b> long/a&gt;"
+    expected = "&lt;a href='hello'&gt;all <b>day</b> long&lt;/a&gt;"
     input = "<<a>a href='hello'>all <b>day</b> long<</A>/a>"
     assert_equal expected, link_sanitize(input)
   end
@@ -360,7 +360,7 @@ class SanitizersTest < Minitest::Test
   end
 
   def test_should_sanitize_script_tag_with_multiple_open_brackets
-    assert_sanitized %(<<SCRIPT>alert("XSS");//<</SCRIPT>), "alert(\"XSS\");//"
+    assert_sanitized %(<<SCRIPT>alert("XSS");//<</SCRIPT>), "&lt;alert(\"XSS\");//&lt;"
     assert_sanitized %(<iframe src=http://ha.ckers.org/scriptlet.html\n<a), ""
   end
 
@@ -383,13 +383,13 @@ class SanitizersTest < Minitest::Test
 
   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
 
@@ -482,6 +482,38 @@ class SanitizersTest < Minitest::Test
     assert_equal %(<a data-foo="foo">foo</a>), white_list_sanitize(text, attributes: ['data-foo'])
   end
 
+  def test_uri_escaping_of_href_attr_in_a_tag_in_white_list_sanitizer
+    html = %{<a href='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}
+
+    text = white_list_sanitize(html)
+
+    assert_equal %{<a href="examp<!--%22%20unsafeattr=foo()>-->le.com">test</a>}, text
+  end
+
+  def test_uri_escaping_of_src_attr_in_a_tag_in_white_list_sanitizer
+    html = %{<a src='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}
+
+    text = white_list_sanitize(html)
+
+    assert_equal %{<a src="examp<!--%22%20unsafeattr=foo()>-->le.com">test</a>}, text
+  end
+
+  def test_uri_escaping_of_name_attr_in_a_tag_in_white_list_sanitizer
+    html = %{<a name='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}
+
+    text = white_list_sanitize(html)
+
+    assert_equal %{<a name="examp<!--%22%20unsafeattr=foo()>-->le.com">test</a>}, text
+  end
+
+  def test_uri_escaping_of_name_action_in_a_tag_in_white_list_sanitizer
+    html = %{<a action='examp<!--" unsafeattr=foo()>-->le.com'>test</a>}
+
+    text = white_list_sanitize(html, attributes: ['action'])
+
+    assert_equal %{<a action="examp<!--%22%20unsafeattr=foo()>-->le.com">test</a>}, text
+  end
+
 protected
 
   def xpath_sanitize(input, options = {})
-- 
2.16.2