diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index df25ec800faddd99f8cd967784f7efdb8381be3f..b684148f91bf888800108bce3e400a027e04a131 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,10 @@ *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] * Fixed JavaScriptHelper#escape_javascript to also escape closing tags #8023 [rubyruy] diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 14c42ce85516184b8f232323b85cb18acbe092eb..8e778f6830505da7091d8c5b80d3def33870e1f0 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -198,6 +198,135 @@ class Base @@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 = /:|(�*58)|(p)|(%|%)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 diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index e7a6303154350c9d081336ce6705667055c02c2e..af6f6e4bb89d874e47bd250630ac3d08864fd906 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -324,63 +324,118 @@ def auto_link(text, link = :all, href_options = {}, &block) # # strip_links('Blog: Visit.') # # => Blog: Visit - def strip_links(text) - text.gsub(/(.*?)<\/a>/mi, '\1') + def strip_links(html) + # Stupid firefox treats 'something' as link! + if html.index(" and ') - # # => <script> do_nasty_stuff() </script> + # <%= sanitize @article.body %> + # + # You can add or remove tags/attributes if you want to customize it a bit. See ActionView::Base for full docs on the + # available options. You can add tags/attributes for single uses of #sanitize by passing either the :attributes or :tags options: # - # sanitize('Click here for $100') - # # => Click here for $100 + # Normal Use # - # sanitize('Click here!!!') - # # => Click here!!! + # <%= sanitize @article.body %> # - # sanitize('') - # # => - def sanitize(html) - # only do this if absolutely necessary - if html.index("<") + # Custom Use + # + # <%= sanitize @article.body, :tags => %w(table tr td), :attributes => %w(id class style) + # + # Add table tags + # + # Rails::Initializer.run do |config| + # config.action_view.sanitized_allowed_tags = 'table', 'tr', 'td' + # end + # + # Remove tags + # + # Rails::Initializer.run do |config| + # config.after_initialize do + # ActionView::Base.sanitized_allowed_tags.delete 'div' + # end + # end + # + # Change allowed attributes + # + # Rails::Initializer.run do |config| + # config.action_view.sanitized_allowed_attributes = 'id', 'class', 'style' + # end + # + def sanitize(html, options = {}) + return html if html.blank? || !html.include?('<') + attrs = options.key?(:attributes) ? Set.new(options[:attributes]).merge(sanitized_allowed_attributes) : sanitized_allowed_attributes + tags = options.key?(:tags) ? Set.new(options[:tags] ).merge(sanitized_allowed_tags) : sanitized_allowed_tags + returning [] do |new_text| tokenizer = HTML::Tokenizer.new(html) - new_text = "" - + parent = [] while token = tokenizer.next node = HTML::Node.parse(nil, 0, 0, token, false) new_text << case node when HTML::Tag - if VERBOTEN_TAGS.include?(node.name) - node.to_s.gsub(/[\n]?/m, "") + strip_tags(text.gsub(/[\n]?/m, "")) # Recurse - handle all dirty nested tags else html # already plain text end @@ -574,6 +629,11 @@ def auto_link_email_addresses(text) end end end + + def contains_bad_protocols?(attr_name, value) + sanitized_uri_attributes.include?(attr_name) && + (value =~ /(^[^\/:]*):|(�*58)|(p)|(%|%)3A/ && !sanitized_allowed_protocols.include?(value.split(sanitized_protocol_separator).first)) + end end end end diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 822b88adee2d0ae7ab81240d9b57d80378ea97ab..80b9c773b3e7c93a5335c13ba4d201c255fba5fd 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -5,7 +5,7 @@ class TextHelperTest < Test::Unit::TestCase include ActionView::Helpers::TextHelper include ActionView::Helpers::TagHelper include TestingSandbox - + def setup # This simulates the fact that instance variables are reset every time # a view is rendered. The cycle helper depends on this behavior. @@ -47,7 +47,13 @@ def test_truncate_multibyte end def test_strip_links + assert_equal "Dont touch me", strip_links("Dont touch me") assert_equal "on my mind\nall day long", strip_links("on my mind\nall day long") + assert_equal "0wn3d", strip_links("0wn3d") + assert_equal "Magic", strip_links("Magic") + assert_equal "FrrFox", strip_links("FrrFox") + assert_equal "My mind\nall day long", strip_links("My mind\nall day long") + assert_equal "all day long", strip_links("<a href='hello'>all day long</a>") end def test_highlighter @@ -255,41 +261,198 @@ def test_auto_link_with_block end def test_sanitize_form - raw = "
" - result = sanitize(raw) - assert_equal %(<form action="/foo/bar" method="post"></form>), result + assert_sanitized "
", '' end def test_sanitize_plaintext raw = "<span>foo</span></plaintext>" - result = sanitize(raw) - assert_equal "&lt;plaintext><span>foo</span>&lt;/plaintext>", result + assert_sanitized raw, "<span>foo</span>" end def test_sanitize_script - raw = "<script language=\"Javascript\">blah blah blah</script>" - result = sanitize(raw) - assert_equal %{&lt;script language="Javascript">blah blah blah&lt;/script>}, result + raw = "a b c<script language=\"Javascript\">blah blah blah</script>d e f" + assert_sanitized raw, "a b cd e f" end def test_sanitize_js_handlers raw = %{onthis="do that" <a href="#" onclick="hello" name="foo" onbogus="remove me">hello</a>} - result = sanitize(raw) - assert_equal %{onthis="do that" <a name="foo" href="#">hello</a>}, result + assert_sanitized raw, %{onthis="do that" <a name="foo" href="#">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>} - result = sanitize(raw) - assert_equal %{href="javascript:bang" <a name="hello">foo</a>, <span>bar</span>}, result + assert_sanitized raw, %{href="javascript:bang" <a name="hello">foo</a>, <span>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>} - result = sanitize(raw) - assert_equal %{src="javascript:bang" <img width="5">foo</img>, <span>bar</span>}, result + assert_sanitized raw, %{src="javascript:bang" <img width="5">foo</img>, <span>bar</span>} + end + + ActionView::Base.sanitized_allowed_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) + end + end + + def test_should_allow_anchors + assert_sanitized %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href="foo"></a>) + end + + # RFC 3986, sec 4.2 + def test_allow_colons_in_path_component + assert_sanitized("<a href=\"./this:that\">foo</a>") + end + + %w(src width height alt).each do |img_attr| + define_method "test_should_allow_image_#{img_attr}_attribute" do + assert_sanitized %(<img #{img_attr}="foo" onclick="bar" />), %(<img #{img_attr}="foo" />) + end + end + + def test_should_handle_non_html + assert_sanitized 'abc' + end + + def test_should_handle_blank_text + assert_sanitized nil + assert_sanitized '' + end + + def test_should_allow_custom_tags + text = "<u>foo</u>" + assert_equal(text, sanitize(text, :tags => %w(u))) + end + + def test_should_allow_custom_tags_with_attributes + text = %(<fieldset foo="bar">foo</fieldset>) + assert_equal(text, sanitize(text, :attributes => ['foo'])) + end + + [%w(img src), %w(a href)].each do |(tag, attr)| + define_method "test_should_strip_#{attr}_attribute_in_#{tag}_with_bad_protocols" do + assert_sanitized %(<#{tag} #{attr}="javascript:bang" title="1">boo</#{tag}>), %(<#{tag} title="1">boo</#{tag}>) + end + end + + def test_should_flag_bad_protocols + %w(about chrome data disk hcp help javascript livescript lynxcgi lynxexec ms-help ms-its mhtml mocha opera res resource shell vbscript view-source vnd.ms.radio wysiwyg).each do |proto| + assert contains_bad_protocols?('src', "#{proto}://bad") + end + end + + def test_should_accept_good_protocols + sanitized_allowed_protocols.each do |proto| + assert !contains_bad_protocols?('src', "#{proto}://good") + end + end + + def test_should_reject_hex_codes_in_protocol + assert contains_bad_protocols?('src', "%6A%61%76%61%73%63%72%69%70%74%3A%61%6C%65%72%74%28%22%58%53%53%22%29") + assert_sanitized %(<a href="&#37;6A&#37;61&#37;76&#37;61&#37;73&#37;63&#37;72&#37;69&#37;70&#37;74&#37;3A&#37;61&#37;6C&#37;65&#37;72&#37;74&#37;28&#37;22&#37;58&#37;53&#37;53&#37;22&#37;29">1</a>), "<a>1</a>" + end + + def test_should_block_script_tag + assert_sanitized %(<SCRIPT\nSRC=http://ha.ckers.org/xss.js></SCRIPT>), "" + end + + [%(<IMG SRC="javascript:alert('XSS');">), + %(<IMG SRC=javascript:alert('XSS')>), + %(<IMG SRC=JaVaScRiPt:alert('XSS')>), + %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">), + %(<IMG SRC=javascript:alert(&quot;XSS&quot;)>), + %(<IMG SRC=javascript:alert(String.fromCharCode(88,83,83))>), + %(<IMG SRC=&#106;&#97;&#118;&#97;&#115;&#99;&#114;&#105;&#112;&#116;&#58;&#97;&#108;&#101;&#114;&#116;&#40;&#39;&#88;&#83;&#83;&#39;&#41;>), + %(<IMG SRC=&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041>), + %(<IMG SRC=&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>), + %(<IMG SRC="jav\tascript:alert('XSS');">), + %(<IMG SRC="jav&#x09;ascript:alert('XSS');">), + %(<IMG SRC="jav&#x0A;ascript:alert('XSS');">), + %(<IMG SRC="jav&#x0D;ascript:alert('XSS');">), + %(<IMG SRC=" &#14; javascript:alert('XSS');">), + %(<IMG SRC=`javascript:alert("RSnake says, 'XSS'")`>)].each_with_index do |img_hack, i| + define_method "test_should_not_fall_for_xss_image_hack_#{i+1}" do + assert_sanitized img_hack, "<img>" + end + end + + def test_should_sanitize_tag_broken_up_by_null + assert_sanitized %(<SCR\0IPT>alert(\"XSS\")</SCR\0IPT>), "alert(\"XSS\")" + end + + def test_should_sanitize_invalid_script_tag + assert_sanitized %(<SCRIPT/XSS SRC="http://ha.ckers.org/xss.js"></SCRIPT>), "" end + def test_should_sanitize_script_tag_with_multiple_open_brackets + assert_sanitized %(<<SCRIPT>alert("XSS");//<</SCRIPT>), "&lt;" + assert_sanitized %(<iframe src=http://ha.ckers.org/scriptlet.html\n<a), %(&lt;a) + end + + def test_should_sanitize_unclosed_script + assert_sanitized %(<SCRIPT SRC=http://ha.ckers.org/xss.js?<B>), "<b>" + end + + def test_should_sanitize_half_open_scripts + assert_sanitized %(<IMG SRC="javascript:alert('XSS')"), "<img>" + end + + def test_should_not_fall_for_ridiculous_hack + img_hack = %(<IMG\nSRC\n=\n"\nj\na\nv\na\ns\nc\nr\ni\np\nt\n:\na\nl\ne\nr\nt\n(\n'\nX\nS\nS\n'\n)\n"\n>) + assert_sanitized img_hack, "<img>" + end + + def test_should_sanitize_attributes + assert_sanitized %(<SPAN title="'><script>alert()</script>">blah</SPAN>), %(<span title="'&gt;&lt;script&gt;alert()&lt;/script&gt;">blah</span>) + end + + 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-image: ; background-x: center; background-y: center;) + assert_equal expected, sanitize_css(raw) + end + + def test_should_sanitize_xul_style_attributes + raw = %(-moz-binding:url('http://ha.ckers.org/xssmoz.xml#xss')) + assert_equal '', sanitize_css(raw) + end + + def test_should_sanitize_invalid_tag_names + assert_sanitized(%(a b c<script/XSS src="http://ha.ckers.org/xss.js"></script>d e f), "a b cd e f") + end + + def test_should_sanitize_non_alpha_and_non_digit_characters_in_tags + assert_sanitized('<a onclick!#$%&()*~+-_.,:;?@[/|\]^`=alert("XSS")>foo</a>', "<a>foo</a>") + end + + def test_should_sanitize_invalid_tag_names_in_single_tags + assert_sanitized('<img/src="http://ha.ckers.org/xss.js"/>', "<img />") + end + + def test_should_sanitize_img_dynsrc_lowsrc + assert_sanitized(%(<img lowsrc="javascript:alert('XSS')" />), "<img />") + end + + def test_should_sanitize_div_background_image_unicode_encoded + raw = %(background-image:\0075\0072\006C\0028'\006a\0061\0076\0061\0073\0063\0072\0069\0070\0074\003a\0061\006c\0065\0072\0074\0028.1027\0058.1053\0053\0027\0029'\0029) + assert_equal '', sanitize_css(raw) + end + + def test_should_sanitize_div_style_expression + raw = %(width: expression(alert('XSS'));) + assert_equal '', sanitize_css(raw) + end + + def test_should_sanitize_style_attribute + raw = %(<div style="display:block; background:url(http://rubyonrails.com); background-image: url(rubyonrails)">foo</div>) + assert_equal %(<div style="display: block; background: ; background-image: ;">foo</div>), sanitize(raw, :attributes => 'style') + end + + def test_should_sanitize_img_vbscript + assert_sanitized %(<img src='vbscript:msgbox("XSS")' />), '<img />' + end + + def test_cycle_class value = Cycle.new("one", 2, "3") assert_equal("one", value.to_s) @@ -374,7 +537,9 @@ def test_cycle_no_instance_variable_clashes end 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( %{This is a test.\n\n\nIt no longer contains any HTML.\n}, strip_tags( @@ -382,4 +547,15 @@ def test_strip_tags assert_equal "This has a here.", strip_tags("This has a <!-- comment --> here.") [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