Merge pull request #116 from rails/115-sanitize-processing-instructions

PermitScrubber does not permit Processing Instructions

Author: flavorjones

.travis.yml

@@ -7,6 +7,8 @@ rvm:
   - 2.4
   - 2.5
   - 2.6
+  - 2.7
+  - 3.0
   - ruby-head
   - jruby
 matrix:

CHANGELOG.md

@@ -1,3 +1,15 @@
+## next / unreleased
+
+* Processing Instructions are no longer allowed by Rails::Html::PermitScrubber
+
+  Previously, a PI with a name (or "target") matching an allowed tag name was not scrubbed. There
+  are no known security issues associated with these PIs, but similar to comments it's preferred to
+  omit these nodes when possible from sanitized output.
+
+  Fixes #115.
+
+  *Mike Dalessio*
+
 ## 1.3.0
 
 * Address deprecations in Loofah 2.3.0.

lib/rails/html/scrubbers.rb

@@ -68,7 +68,7 @@ def scrub(node)
         end
         return CONTINUE if skip_node?(node)
 
-        unless keep_node?(node)
+        unless node.element? && keep_node?(node)
           return STOP if scrub_node(node) == STOP
         end
 

test/sanitizer_test.rb

@@ -521,6 +521,14 @@ def test_uri_escaping_of_name_action_in_a_tag_in_safe_list_sanitizer
     assert_equal %{<a action=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>}, text
   end
 
+  def test_exclude_node_type_processing_instructions
+    assert_equal("<div>text</div><b>text</b>", safe_list_sanitize("<div>text</div><?div content><b>text</b>"))
+  end
+
+  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
+
 protected
 
   def xpath_sanitize(input, options = {})