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

lib/rails/html/sanitizer.rb

@@ -97,6 +97,10 @@ 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

lib/rails/html/scrubbers.rb

@@ -100,6 +100,7 @@ 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)
@@ -123,6 +124,30 @@ def validate!(var, name)
         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

test/sanitizer_test.rb

@@ -152,7 +152,7 @@ def test_sanitize_plaintext
   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
@@ -173,17 +173,23 @@ def test_sanitize_image_src
   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
@@ -309,7 +315,7 @@ def test_should_block_script_tag
   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');">),
@@ -453,6 +459,16 @@ def test_sanitize_ascii_8bit_string
     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 = {})
@@ -484,18 +500,18 @@ def 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