CVE-2015-7578: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in the rails-html-sanitizer gem before 1.0.3 for Ruby on Rails 4.2.x and 5.x allows remote attackers to inject arbitrary web script or HTML via crafted tag attributes.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2016-02-16T02:59Z
Repository: https://github.com/rails/rails-html-sanitizer
Language: Ruby
Files changed: lib/rails/html/sanitizer.rb, lib/rails/html/scrubbers.rb, test/sanitizer_test.rb
Commit date: 2016-01-22 15:07:39 -0800
Commit message: Define a less permissive list of tags and attributes

And use it by default.

The new sanitizer were being a lot more permissive that we had in Rails
until the version 4.2.

This was also allowing arbritary data attributes by default what can
lead to CSRF and XSS attacks.

Now data attributes need to be explicitly allowed.

CVE-2015-7578

Diff: diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb
index 8be10d5..f40bf6b 100644
--- a/lib/rails/html/sanitizer.rb
+++ b/lib/rails/html/sanitizer.rb
@@ -95,10 +95,14 @@ module Rails
     class WhiteListSanitizer < Sanitizer
       class << self
         attr_accessor :allowed_tags
         attr_accessor :allowed_attributes
       end
+      self.allowed_tags = Set.new(%w(strong em b i p code pre tt samp kbd var sub
+        sup dfn cite big small address hr br div span h1 h2 h3 h4 h5 h6 ul ol li dl dt dd abbr
+        acronym a img blockquote del ins))
+      self.allowed_attributes = Set.new(%w(href src width height alt cite datetime title class name xml:lang abbr))
 
       def initialize
         @permit_scrubber = PermitScrubber.new
       end
 
diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 1384a2f..2c89c04 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -98,10 +98,11 @@ module Rails
 
       def scrub_attributes(node)
         if @attributes
           node.attribute_nodes.each do |attr|
             attr.remove if scrub_attribute?(attr.name)
+            scrub_attribute(node, attr)
           end
 
           scrub_css_attribute(node)
         else
           Loofah::HTML5::Scrub.scrub_attributes(node)
@@ -121,10 +122,34 @@ module Rails
         if var && !var.is_a?(Enumerable)
           raise ArgumentError, "You should pass :#{name} as an Enumerable"
         end
         var
       end
+
+      def scrub_attribute(node, attr_node)
+        attr_name = if attr_node.namespace
+                      "#{attr_node.namespace.prefix}:#{attr_node.node_name}"
+                    else
+                      attr_node.node_name
+                    end
+
+        if Loofah::HTML5::WhiteList::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::WhiteList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(Loofah::HTML5::WhiteList::PROTOCOL_SEPARATOR)[0])
+            attr_node.remove
+          end
+        end
+        if Loofah::HTML5::WhiteList::SVG_ATTR_VAL_ALLOWS_REF.include?(attr_name)
+          attr_node.value = attr_node.value.gsub(/url\s*\(\s*[^#\s][^)]+?\)/m, ' ') if attr_node.value
+        end
+        if Loofah::HTML5::WhiteList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == 'xlink:href' && attr_node.value =~ /^\s*[^#\s].*/m
+          attr_node.remove
+        end
+
+        node.remove_attribute(attr_node.name) if attr_name == 'src' && attr_node.value !~ /[^[:space:]]/
+      end
     end
 
     # === Rails::Html::TargetScrubber
     #
     # Where Rails::Html::PermitScrubber picks out tags and attributes to permit in
diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb
index c4e5616..58c0054 100644
--- a/test/sanitizer_test.rb
+++ b/test/sanitizer_test.rb
@@ -150,11 +150,11 @@ class SanitizersTest < Minitest::Test
   def test_sanitize_plaintext
     assert_sanitized "<plaintext><span>foo</span></plaintext>", "<span>foo</span>"
   end
 
   def test_sanitize_script
-    assert_sanitized "a b c<script language=\"Javascript\">blah blah blah</script>d e f", "a b cd e f"
+    assert_sanitized "a b c<script language=\"Javascript\">blah blah blah</script>d e f", "a b cblah blah blahd e f"
   end
 
   def test_sanitize_js_handlers
     raw = %{onthis="do that" <a href="#" onclick="hello" name="foo" onbogus="remove me">hello</a>}
     assert_sanitized raw, %{onthis="do that" <a href="#" name="foo">hello</a>}
@@ -171,21 +171,27 @@ class SanitizersTest < Minitest::Test
   end
 
   tags = Loofah::HTML5::WhiteList::ALLOWED_ELEMENTS - %w(script form)
   tags.each do |tag_name|
     define_method "test_should_allow_#{tag_name}_tag" do
-      assert_sanitized "start <#{tag_name} title=\"1\" onclick=\"foo\">foo <bad>bar</bad> baz</#{tag_name}> end", %(start <#{tag_name} title="1">foo bar baz</#{tag_name}> end)
+      scope_allowed_tags(tags) do
+        assert_sanitized "start <#{tag_name} title=\"1\" onclick=\"foo\">foo <bad>bar</bad> baz</#{tag_name}> end", %(start <#{tag_name} title="1">foo bar baz</#{tag_name}> end)
+      end
     end
   end
 
   def test_should_allow_anchors
-    assert_sanitized %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href=\"foo\"></a>)
+    assert_sanitized %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href=\"foo\">baz</a>)
   end
 
   def test_video_poster_sanitization
-    assert_sanitized %(<video src="videofile.ogg" autoplay  poster="posterimage.jpg"></video>), %(<video src="videofile.ogg" poster="posterimage.jpg"></video>)
-    assert_sanitized %(<video src="videofile.ogg" poster=javascript:alert(1)></video>), %(<video src="videofile.ogg"></video>)
+    scope_allowed_tags(%w(video)) do
+      scope_allowed_attributes %w(src poster) do
+        assert_sanitized %(<video src="videofile.ogg" autoplay  poster="posterimage.jpg"></video>), %(<video src="videofile.ogg" poster="posterimage.jpg"></video>)
+        assert_sanitized %(<video src="videofile.ogg" poster=javascript:alert(1)></video>), %(<video src="videofile.ogg"></video>)
+      end
+    end
   end
 
   # RFC 3986, sec 4.2
   def test_allow_colons_in_path_component
     assert_sanitized "<a href=\"./this:that\">foo</a>"
@@ -307,11 +313,11 @@ class SanitizersTest < Minitest::Test
   def test_should_block_script_tag
     assert_sanitized %(<SCRIPT\nSRC=http://ha.ckers.org/xss.js></SCRIPT>), ""
   end
 
   def test_should_not_fall_for_xss_image_hack_with_uppercase_tags
-    assert_sanitized %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">), "<img>\"&gt;"
+    assert_sanitized %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">), %(<img>alert("XSS")"&gt;)
   end
 
   [%(<IMG SRC="javascript:alert('XSS');">),
    %(<IMG SRC=javascript:alert('XSS')>),
    %(<IMG SRC=JaVaScRiPt:alert('XSS')>),
@@ -451,10 +457,20 @@ class SanitizersTest < Minitest::Test
       assert_equal '<a>hello</a>', sanitized
       assert_equal Encoding::UTF_8, sanitized.encoding
     end
   end
 
+  def test_sanitize_data_attributes
+    assert_sanitized %(<a href="/blah" data-method="post">foo</a>), %(<a href="/blah">foo</a>)
+    assert_sanitized %(<a data-remote="true" data-type="script" data-method="get" data-cross-domain="true" href="attack.js">Launch the missiles</a>), %(<a href="attack.js">Launch the missiles</a>)
+  end
+
+  def test_allow_data_attribute_if_requested
+    text = %(<a data-foo="foo">foo</a>)
+    assert_equal %(<a data-foo="foo">foo</a>), white_list_sanitize(text, attributes: ['data-foo'])
+  end
+
 protected
 
   def xpath_sanitize(input, options = {})
     XpathRemovalTestSanitizer.new.sanitize(input, options)
   end
@@ -482,20 +498,20 @@ protected
   def sanitize_css(input)
     Rails::Html::WhiteListSanitizer.new.sanitize_css(input)
   end
 
   def scope_allowed_tags(tags)
+    old_tags = Rails::Html::WhiteListSanitizer.allowed_tags
     Rails::Html::WhiteListSanitizer.allowed_tags = tags
     yield Rails::Html::WhiteListSanitizer.new
-
   ensure
-    Rails::Html::WhiteListSanitizer.allowed_tags = nil
+    Rails::Html::WhiteListSanitizer.allowed_tags = old_tags
   end
 
   def scope_allowed_attributes(attributes)
+    old_attributes = Rails::Html::WhiteListSanitizer.allowed_attributes
     Rails::Html::WhiteListSanitizer.allowed_attributes = attributes
     yield Rails::Html::WhiteListSanitizer.new
-
   ensure
-    Rails::Html::WhiteListSanitizer.allowed_attributes = nil
+    Rails::Html::WhiteListSanitizer.allowed_attributes = old_attributes
   end
 end

Vulnerable code:     assert_sanitized "a b c<script language=\"Javascript\">blah blah blah</script>d e f", "a b cd e f"
      assert_sanitized "start <#{tag_name} title=\"1\" onclick=\"foo\">foo <bad>bar</bad> baz</#{tag_name}> end", %(start <#{tag_name} title="1">foo bar baz</#{tag_name}> end)
    assert_sanitized %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href=\"foo\"></a>)
    assert_sanitized %(<video src="videofile.ogg" autoplay  poster="posterimage.jpg"></video>), %(<video src="videofile.ogg" poster="posterimage.jpg"></video>)
    assert_sanitized %(<video src="videofile.ogg" poster=javascript:alert(1)></video>), %(<video src="videofile.ogg"></video>)
    assert_sanitized %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">), "<img>\"&gt;"

    Rails::Html::WhiteListSanitizer.allowed_tags = nil

    Rails::Html::WhiteListSanitizer.allowed_attributes = nil
Fixed code:       self.allowed_tags = Set.new(%w(strong em b i p code pre tt samp kbd var sub
        sup dfn cite big small address hr br div span h1 h2 h3 h4 h5 h6 ul ol li dl dt dd abbr
        acronym a img blockquote del ins))
      self.allowed_attributes = Set.new(%w(href src width height alt cite datetime title class name xml:lang abbr))
            scrub_attribute(node, attr)

      def scrub_attribute(node, attr_node)
        attr_name = if attr_node.namespace
                      "#{attr_node.namespace.prefix}:#{attr_node.node_name}"
                    else
                      attr_node.node_name
                    end

        if Loofah::HTML5::WhiteList::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::WhiteList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(Loofah::HTML5::WhiteList::PROTOCOL_SEPARATOR)[0])
            attr_node.remove
          end
        end
        if Loofah::HTML5::WhiteList::SVG_ATTR_VAL_ALLOWS_REF.include?(attr_name)
          attr_node.value = attr_node.value.gsub(/url\s*\(\s*[^#\s][^)]+?\)/m, ' ') if attr_node.value
        end
        if Loofah::HTML5::WhiteList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == 'xlink:href' && attr_node.value =~ /^\s*[^#\s].*/m
          attr_node.remove
        end

        node.remove_attribute(attr_node.name) if attr_name == 'src' && attr_node.value !~ /[^[:space:]]/
      end
    assert_sanitized "a b c<script language=\"Javascript\">blah blah blah</script>d e f", "a b cblah blah blahd e f"
      scope_allowed_tags(tags) do
        assert_sanitized "start <#{tag_name} title=\"1\" onclick=\"foo\">foo <bad>bar</bad> baz</#{tag_name}> end", %(start <#{tag_name} title="1">foo bar baz</#{tag_name}> end)
      end
    assert_sanitized %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href=\"foo\">baz</a>)
    scope_allowed_tags(%w(video)) do
      scope_allowed_attributes %w(src poster) do
        assert_sanitized %(<video src="videofile.ogg" autoplay  poster="posterimage.jpg"></video>), %(<video src="videofile.ogg" poster="posterimage.jpg"></video>)
        assert_sanitized %(<video src="videofile.ogg" poster=javascript:alert(1)></video>), %(<video src="videofile.ogg"></video>)
      end
    end
    assert_sanitized %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">), %(<img>alert("XSS")"&gt;)
  def test_sanitize_data_attributes
    assert_sanitized %(<a href="/blah" data-method="post">foo</a>), %(<a href="/blah">foo</a>)
    assert_sanitized %(<a data-remote="true" data-type="script" data-method="get" data-cross-domain="true" href="attack.js">Launch the missiles</a>), %(<a href="attack.js">Launch the missiles</a>)
  end

  def test_allow_data_attribute_if_requested
    text = %(<a data-foo="foo">foo</a>)
    assert_equal %(<a data-foo="foo">foo</a>), white_list_sanitize(text, attributes: ['data-foo'])
  end

    old_tags = Rails::Html::WhiteListSanitizer.allowed_tags
    Rails::Html::WhiteListSanitizer.allowed_tags = old_tags
    old_attributes = Rails::Html::WhiteListSanitizer.allowed_attributes
    Rails::Html::WhiteListSanitizer.allowed_attributes = old_attributes