Changeset 7589

Timestamp:

09/23/07 00:11:08 (8 months ago)

Author:

rick

Message:

Secure #sanitize, #strip_tags, and #strip_links helpers against xss attacks. Closes #8877. [Rick, lifofifo, Jacques Distler]

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_view/base.rb (modified) (1 diff)
• trunk/actionpack/lib/action_view/helpers/text_helper.rb (modified) (3 diffs)
• trunk/actionpack/test/template/text_helper_test.rb (modified) (5 diffs)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Secure #sanitize, #strip_tags, and #strip_links helpers against xss attacks.  Closes #8877. [Rick, lifofifo, Jacques Distler]
+
+  This merges and renames the popular white_list helper (along with some css sanitizing from Jacques Distler version of the same plugin).
+  Also applied updated versions of #strip_tags and #strip_links from #8877.
-
-* Remove use of & logic operator. Closes #8114. [watson]

trunk/actionpack/lib/action_view/base.rb

@@ -199 +199 @@
-    @@erb_variable = '_erbout'
-    cattr_accessor :erb_variable
+    
+    # A regular expression of the valid characters used to separate protocols like
+    # the ':' in 'http://foo.com'
+    @@sanitized_protocol_separator = /:|(&#0*58)|(&#x70)|(%|%)3A/
+    cattr_accessor :sanitized_protocol_separator
+
+    # Specifies a Set of HTML attributes that can have URIs.
+    @@sanitized_uri_attributes = Set.new(%w(href src cite action longdesc xlink:href lowsrc))
+    cattr_reader :sanitized_uri_attributes
+
+    # Adds valid HTML attributes that the #sanitize helper checks for URIs.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_uri_attributes = 'lowsrc', 'target'
+    #   end
+    #
+    def self.sanitized_uri_attributes=(attributes)
+      @@sanitized_uri_attributes.merge(attributes)
+    end
+
+    # Specifies a Set of 'bad' tags that the #sanitize helper will remove completely, as opposed
+    # to just escaping harmless tags like <font>
+    @@sanitized_bad_tags = Set.new('script')
+    cattr_reader :sanitized_bad_tags
+    
+    # Adds to the Set of 'bad' tags for the #sanitize helper.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_bad_tags = 'embed', 'object'
+    #   end
+    #
+    def self.sanitized_bad_tags=(attributes)
+      @@sanitized_bad_tags.merge(attributes)
+    end
+    
+    # Specifies the default Set of tags that the #sanitize helper will allow unscathed.
+    @@sanitized_allowed_tags = Set.new(%w(strong em b i p code pre tt output samp kbd var sub 
+      sup dfn cite big small address hr br div span h1 h2 h3 h4 h5 h6 ul ol li dt dd abbr 
+      acronym a img blockquote del ins fieldset legend))
+    cattr_reader :sanitized_allowed_tags
+
+    # Adds to the Set of allowed tags for the #sanitize helper.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_allowed_tags = 'table', 'tr', 'td'
+    #   end
+    #
+    def self.sanitized_allowed_tags=(attributes)
+      @@sanitized_allowed_tags.merge(attributes)
+    end
+
+    # Specifies the default Set of html attributes that the #sanitize helper will leave 
+    # in the allowed tag.
+    @@sanitized_allowed_attributes = Set.new(%w(href src width height alt cite datetime title class name xml:lang abbr))
+    cattr_reader :sanitized_allowed_attributes
+
+    # Adds to the Set of allowed html attributes for the #sanitize helper.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_allowed_attributes = 'onclick', 'longdesc'
+    #   end
+    #
+    def self.sanitized_allowed_attributes=(attributes)
+      @@sanitized_allowed_attributes.merge(attributes)
+    end
+
+    # Specifies the default Set of acceptable css properties that #sanitize and #sanitize_css will accept.
+    @@sanitized_allowed_css_properties = Set.new(%w(azimuth background-color border-bottom-color border-collapse 
+      border-color border-left-color border-right-color border-top-color clear color cursor direction display 
+      elevation float font font-family font-size font-style font-variant font-weight height letter-spacing line-height
+      overflow pause pause-after pause-before pitch pitch-range richness speak speak-header speak-numeral speak-punctuation
+      speech-rate stress text-align text-decoration text-indent unicode-bidi vertical-align voice-family volume white-space
+      width))
+    cattr_reader :sanitized_allowed_css_properties
+
+    # Adds to the Set of allowed css properties for the #sanitize and #sanitize_css heleprs.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_allowed_css_properties = 'expression'
+    #   end
+    #
+    def self.sanitized_allowed_css_properties=(attributes)
+      @@sanitized_allowed_css_properties.merge(attributes)
+    end
+    
+    # Specifies the default Set of acceptable css keywords that #sanitize and #sanitize_css will accept.
+    @@sanitized_allowed_css_keywords = Set.new(%w(auto aqua black block blue bold both bottom brown center
+      collapse dashed dotted fuchsia gray green !important italic left lime maroon medium none navy normal
+      nowrap olive pointer purple red right solid silver teal top transparent underline white yellow))
+    cattr_reader :sanitized_allowed_css_keywords
+
+    # Adds to the Set of allowed css keywords for the #sanitize and #sanitize_css helpers.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_allowed_css_keywords = 'expression'
+    #   end
+    #
+    def self.sanitized_allowed_css_keywords=(attributes)
+      @@sanitized_allowed_css_keywords.merge(attributes)
+    end
+    
+    # Specifies the default Set of allowed shorthand css properties for the #sanitize and #sanitize_css helpers.
+    @@sanitized_shorthand_css_properties = Set.new(%w(background border margin padding))
+    cattr_reader :sanitized_shorthand_css_properties
+
+    # Adds to the Set of allowed shorthand css properties for the #sanitize and #sanitize_css helpers.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_shorthand_css_properties = 'expression'
+    #   end
+    #
+    def self.sanitized_shorthand_css_properties=(attributes)
+      @@sanitized_shorthand_css_properties.merge(attributes)
+    end
+
+    # Specifies the default Set of protocols that the #sanitize helper will leave in
+    # protocol attributes.
+    @@sanitized_allowed_protocols = Set.new(%w(ed2k ftp http https irc mailto news gopher nntp telnet webcal xmpp callto feed svn urn aim rsync tag ssh sftp rtsp afs))
+    cattr_reader :sanitized_allowed_protocols
+
+    # Adds to the Set of allowed protocols for the #sanitize helper.
+    #
+    #   Rails::Initializer.run do |config|
+    #     config.action_view.sanitized_allowed_protocols = 'ssh', 'feed'
+    #   end
+    #
+    def self.sanitized_allowed_protocols=(attributes)
+      @@sanitized_allowed_protocols.merge(attributes)
+    end
-
-    @@template_handlers = HashWithIndifferentAccess.new

trunk/actionpack/lib/action_view/helpers/text_helper.rb

@@ -325 +325 @@
-      #   strip_links('Blog: <a href="http://www.myblog.com/" class="nav" target=\"_blank\">Visit</a>.')
-      #   # => Blog: Visit
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-          tokenizer = HTML::Tokenizer.new(html)
-
-
+
-          while token = tokenizer.next
-            node = HTML::Node.parse(nil, 0, 0, token, false)
-            new_text << case node
-              when HTML::Tag
-VERBOTEN_TAGS.include?(node.name)
-node.to_s.gsub(/</, "&lt;")
+node.closing == :close
+parent.shift
-                else
-
-
-
-
-
+
+
+
+
+
+
+
+
-                  end
-  node.to_
-end
+end if node.attribute
+tags.include?(node.name) ? node : nil
-              else
-
+sanitized_bad_tags.include?(parent.first) ? nil : 
-            end
-          end
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-      # Strips all HTML tags from the +html+, including comments.  This uses the 
-      # html-scanner tokenizer and so its HTML parsing ability is limited by 
@@ -408 +463 @@
-          # strip any comments, and if they have a newline at the end (ie. line with
-          # only a comment) strip that too
-text.gsub(/<!--(.*?)-->[\n]?/m, "") 
+strip_tags(text.gsub(/<!--(.*?)-->[\n]?/m, "")) # Recurse - handle all dirty nested tags
-        else
-          html # already plain text
@@ -575 +630 @@
-          end
-        end
+
+        def contains_bad_protocols?(attr_name, value)
+          sanitized_uri_attributes.include?(attr_name) && 
+          (value =~ /(^[^\/:]*):|(&#0*58)|(&#x70)|(%|&#37;)3A/ && !sanitized_allowed_protocols.include?(value.split(sanitized_protocol_separator).first))
+        end
-    end
-  end

trunk/actionpack/test/template/text_helper_test.rb

@@ -6 +6 @@
-  include ActionView::Helpers::TagHelper
-  include TestingSandbox
-
+
-  def setup
-    # This simulates the fact that instance variables are reset every time
@@ -48 +48 @@
-  
-  def test_strip_links
+    assert_equal "Dont touch me", strip_links("Dont touch me")
-    assert_equal "on my mind\nall day long", strip_links("<a href='almost'>on my mind</a>\n<A href='almost'>all day long</A>")
+    assert_equal "0wn3d", strip_links("<a href='http://www.rubyonrails.com/'><a href='http://www.rubyonrails.com/' onlclick='steal()'>0wn3d</a></a>") 
+    assert_equal "Magic", strip_links("<a href='http://www.rubyonrails.com/'>Mag<a href='http://www.ruby-lang.org/'>ic") 
+    assert_equal "FrrFox", strip_links("<href onlclick='steal()'>FrrFox</a></href>") 
+    assert_equal "My mind\nall <b>day</b> long", strip_links("<a href='almost'>My mind</a>\n<A href='almost'>all <b>day</b> long</A>")
+    assert_equal "all <b>day</b> long", strip_links("<<a>a href='hello'>all <b>day</b> long<</A>/a>")
-  end
-
@@ -256 +262 @@
-
-  def test_sanitize_form
-
-
-
+
-  end
-
-  def test_sanitize_plaintext
-    raw = "<plaintext><span>foo</span></plaintext>"
-
-
+
-  end
-
-  def test_sanitize_script
-
-
-
+
+
-  end
-
-  def test_sanitize_js_handlers
-    raw = %{onthis="do that" <a href="#" onclick="hello" name="foo" onbogus="remove me">hello</a>}
-
-
+
-  end
-
-  def test_sanitize_javascript_href
-    raw = %{href="javascript:bang" <a href="javascript:bang" name="hello">foo</a>, <span href="javascript:bang">bar</span>}
-
-
+
-  end
-  
-  def test_sanitize_image_src
-    raw = %{src="javascript:bang" <img src="javascript:bang" width="5">foo</img>, <span src="javascript:bang">bar</span>}
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-  def test_cycle_class
-    value = Cycle.new("one", 2, "3")
@@ -375 +538 @@
-
-  def test_strip_tags
+    assert_equal("Dont touch me", strip_tags("Dont touch me"))
-    assert_equal("This is a test.", strip_tags("<p>This <u>is<u> a <a href='test.html'><strong>test</strong></a>.</p>"))
+    assert_equal("Weirdos", strip_tags("Wei<<a>a onclick='alert(document.cookie);'</a>/>rdos"))
-    assert_equal("This is a test.", strip_tags("This is a test."))
-    assert_equal(
@@ -383 +548 @@
-    [nil, '', '   '].each { |blank| assert_equal blank, strip_tags(blank) }
-  end
+
+  def assert_sanitized(text, expected = nil)
+    assert_equal((expected || text), sanitize(text))
+  end
+
+  # pull in configuration values from ActionView::Base
+  [:sanitized_protocol_separator, :sanitized_protocol_attributes, :sanitized_bad_tags, :sanitized_allowed_tags, :sanitized_allowed_attributes, :sanitized_allowed_protocols, :sanitized_allowed_css_properties, :sanitized_allowed_css_keywords, :sanitized_shorthand_css_properties, :sanitized_uri_attributes].each do |attr|
+    define_method attr do
+      ActionView::Base.send(attr)
+    end
+  end
-end