html_escape should escape single quotes

https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content Closes #7215
rails · Aug 7, 2012 · d0c9759 · d0c9759
1 parent d314a48
commit d0c9759
Show file tree

Hide file tree

Showing 12 changed files with 51 additions and 72 deletions.
diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb
@@ -120,7 +120,7 @@ class TestWithoutLayout < Rack::TestCase
     test "rendering a template with error properly exceprts the code" do
       get :with_error
       assert_status 500
-      assert_match "undefined local variable or method `idontexist'", response.body
+      assert_match "undefined local variable or method `idontexist", response.body
     end
   end
 

diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
@@ -152,7 +152,7 @@ def render_text_hello_world
 
   # :ported:
   def render_text_hello_world_with_layout
-    @variable_for_layout = ", I'm here!"
+    @variable_for_layout = ", I am here!"
     render :text => "hello world", :layout => true
   end
 
@@ -781,7 +781,7 @@ def test_render_text
   # :ported:
   def test_do_with_render_text_and_layout
     get :render_text_hello_world_with_layout
-    assert_equal "<html>hello world, I'm here!</html>", @response.body
+    assert_equal "<html>hello world, I am here!</html>", @response.body
   end
 
   # :ported:

diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
@@ -162,8 +162,8 @@ def teardown
     %(image_tag(".pdf.png")) => %(<img alt=".pdf" src="/images/.pdf.png" />),
     %(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(<img alt="Rails" src="http://www.rubyonrails.com/images/rails.png" />),
     %(image_tag("//www.rubyonrails.com/images/rails.png")) => %(<img alt="Rails" src="//www.rubyonrails.com/images/rails.png" />),
-    %(image_tag("mouse.png", :mouseover => "/images/mouse_over.png")) => %(<img alt="Mouse" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" src="/images/mouse.png" />),
-    %(image_tag("mouse.png", :mouseover => image_path("mouse_over.png"))) => %(<img alt="Mouse" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" src="/images/mouse.png" />),
+    %(image_tag("mouse.png", :mouseover => "/images/mouse_over.png")) => %(<img alt="Mouse" onmouseover="this.src=&#x27;/images/mouse_over.png&#x27;" onmouseout="this.src=&#x27;/images/mouse.png&#x27;" src="/images/mouse.png" />),
+    %(image_tag("mouse.png", :mouseover => image_path("mouse_over.png"))) => %(<img alt="Mouse" onmouseover="this.src=&#x27;/images/mouse_over.png&#x27;" onmouseout="this.src=&#x27;/images/mouse.png&#x27;" src="/images/mouse.png" />),
     %(image_tag("mouse.png", :alt => nil)) => %(<img src="/images/mouse.png" />),
     %(image_tag("data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==", :alt => nil)) => %(<img src="data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==" />),
   }
@@ -1096,8 +1096,8 @@ def test_should_compute_proper_path
     assert_dom_equal(%(/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr"))
     assert_dom_equal(%(/collaboration/hieraki/stylesheets/style.css), stylesheet_path("style"))
     assert_dom_equal(%(/collaboration/hieraki/images/xml.png), image_path("xml.png"))
-    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src='/collaboration/hieraki/images/mouse_over.png'" onmouseout="this.src='/collaboration/hieraki/images/mouse.png'" src="/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
-    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src='/collaboration/hieraki/images/mouse_over2.png'" onmouseout="this.src='/collaboration/hieraki/images/mouse2.png'" src="/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
+    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src=&#x27;/collaboration/hieraki/images/mouse_over.png&#x27;" onmouseout="this.src=&#x27;/collaboration/hieraki/images/mouse.png&#x27;" src="/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
+    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src=&#x27;/collaboration/hieraki/images/mouse_over2.png&#x27;" onmouseout="this.src=&#x27;/collaboration/hieraki/images/mouse2.png&#x27;" src="/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
   end
 
   def test_should_ignore_relative_root_path_on_complete_url
@@ -1110,8 +1110,8 @@ def test_should_compute_proper_path_with_asset_host
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr"))
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_path("style"))
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png"))
-    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse_over.png'" onmouseout="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse.png'" src="gopher://assets.example.com/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
-    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse_over2.png'" onmouseout="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse2.png'" src="gopher://assets.example.com/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
+    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse_over.png&#x27;" onmouseout="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse.png&#x27;" src="gopher://assets.example.com/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
+    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse_over2.png&#x27;" onmouseout="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse2.png&#x27;" src="gopher://assets.example.com/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
   end
 
   def test_should_compute_proper_path_with_asset_host_and_default_protocol
@@ -1120,8 +1120,8 @@ def test_should_compute_proper_path_with_asset_host_and_default_protocol
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr"))
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_path("style"))
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png"))
-    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse_over.png'" onmouseout="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse.png'" src="gopher://assets.example.com/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
-    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse_over2.png'" onmouseout="this.src='gopher://assets.example.com/collaboration/hieraki/images/mouse2.png'" src="gopher://assets.example.com/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
+    assert_dom_equal(%(<img alt="Mouse" onmouseover="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse_over.png&#x27;" onmouseout="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse.png&#x27;" src="gopher://assets.example.com/collaboration/hieraki/images/mouse.png" />), image_tag("mouse.png", :mouseover => "/images/mouse_over.png"))
+    assert_dom_equal(%(<img alt="Mouse2" onmouseover="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse_over2.png&#x27;" onmouseout="this.src=&#x27;gopher://assets.example.com/collaboration/hieraki/images/mouse2.png&#x27;" src="gopher://assets.example.com/collaboration/hieraki/images/mouse2.png" />), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png")))
   end
 
   def test_should_ignore_asset_host_on_complete_url

diff --git a/actionpack/test/template/erb_util_test.rb b/actionpack/test/template/erb_util_test.rb
@@ -8,11 +8,11 @@ class ErbUtilTest < Test::Unit::TestCase
     define_method "test_html_escape_#{expected.gsub(/\W/, '')}" do
       assert_equal expected, html_escape(given)
     end
+  end
 
-    unless given == '"'
-      define_method "test_json_escape_#{expected.gsub(/\W/, '')}" do
-        assert_equal ERB::Util::JSON_ESCAPE[given], json_escape(given)
-      end
+  ERB::Util::JSON_ESCAPE.each do |given, expected|
+    define_method "test_json_escape_#{expected.gsub(/\W/, '')}" do
+      assert_equal ERB::Util::JSON_ESCAPE[given], json_escape(given)
     end
   end
 
@@ -40,7 +40,7 @@ def test_html_escape_passes_html_escpe_unmodified
 
   def test_rest_in_ascii
     (0..127).to_a.map {|int| int.chr }.each do |chr|
-      next if chr.in?('&"<>')
+      next if chr.in?('&"<>\'')
       assert_equal chr, html_escape(chr)
     end
   end

diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb
@@ -907,7 +907,7 @@ def test_time_zone_select_with_default_time_zone_and_value
 
   def test_options_for_select_with_element_attributes
     assert_dom_equal(
-      "<option value=\"&lt;Denmark&gt;\" class=\"bold\">&lt;Denmark&gt;</option>\n<option value=\"USA\" onclick=\"alert('Hello World');\">USA</option>\n<option value=\"Sweden\">Sweden</option>\n<option value=\"Germany\">Germany</option>",
+      "<option value=\"&lt;Denmark&gt;\" class=\"bold\">&lt;Denmark&gt;</option>\n<option value=\"USA\" onclick=\"alert(&#x27;Hello World&#x27;);\">USA</option>\n<option value=\"Sweden\">Sweden</option>\n<option value=\"Germany\">Germany</option>",
       options_for_select([ [ "<Denmark>", { :class => 'bold' } ], [ "USA", { :onclick => "alert('Hello World');" } ], [ "Sweden" ], "Germany" ])
     )
   end
@@ -943,13 +943,13 @@ def test_option_html_attributes_with_single_element_hash
   def test_option_html_attributes_with_multiple_element_hash
     output = option_html_attributes([ 'foo', 'bar', { :class => 'fancy', 'onclick' => "alert('Hello World');" } ])
     assert output.include?(" class=\"fancy\"")
-    assert output.include?(" onclick=\"alert('Hello World');\"")
+    assert output.include?(" onclick=\"alert(&#x27;Hello World&#x27;);\"")
   end
 
   def test_option_html_attributes_with_multiple_hashes
     output = option_html_attributes([ 'foo', 'bar', { :class => 'fancy' }, { 'onclick' => "alert('Hello World');" } ])
     assert output.include?(" class=\"fancy\"")
-    assert output.include?(" onclick=\"alert('Hello World');\"")
+    assert output.include?(" onclick=\"alert(&#x27;Hello World&#x27;);\"")
   end
 
   def test_option_html_attributes_with_special_characters

diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb
@@ -366,7 +366,7 @@ def test_stringify_symbol_keys
 
   def test_submit_tag
     assert_dom_equal(
-      %(<input name='commit' data-disable-with="Saving..." onclick="alert('hello!')" type="submit" value="Save" />),
+      %(<input name='commit' data-disable-with="Saving..." onclick="alert(&#x27;hello!&#x27;)" type="submit" value="Save" />),
       submit_tag("Save", :disable_with => "Saving...", :onclick => "alert('hello!')")
     )
   end

diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb
@@ -40,12 +40,12 @@ def test_escape_javascript_with_safebuffer
   end
 
   def test_button_to_function
-    assert_dom_equal %(<input type="button" onclick="alert('Hello world!');" value="Greeting" />),
+    assert_dom_equal %(<input type="button" onclick="alert(&#x27;Hello world!&#x27;);" value="Greeting" />),
       button_to_function("Greeting", "alert('Hello world!')")
   end
 
   def test_button_to_function_with_onclick
-    assert_dom_equal "<input onclick=\"alert('Goodbye World :('); alert('Hello world!');\" type=\"button\" value=\"Greeting\" />",
+    assert_dom_equal "<input onclick=\"alert(&#x27;Goodbye World :(&#x27;); alert(&#x27;Hello world!&#x27;);\" type=\"button\" value=\"Greeting\" />",
       button_to_function("Greeting", "alert('Hello world!')", :onclick => "alert('Goodbye World :(')")
   end
 
@@ -55,17 +55,17 @@ def test_button_to_function_without_function
   end
 
   def test_link_to_function
-    assert_dom_equal %(<a href="#" onclick="alert('Hello world!'); return false;">Greeting</a>),
+    assert_dom_equal %(<a href="#" onclick="alert(&#x27;Hello world!&#x27;); return false;">Greeting</a>),
       link_to_function("Greeting", "alert('Hello world!')")
   end
 
   def test_link_to_function_with_existing_onclick
-    assert_dom_equal %(<a href="#" onclick="confirm('Sanity!'); alert('Hello world!'); return false;">Greeting</a>),
+    assert_dom_equal %(<a href="#" onclick="confirm(&#x27;Sanity!&#x27;); alert(&#x27;Hello world!&#x27;); return false;">Greeting</a>),
       link_to_function("Greeting", "alert('Hello world!')", :onclick => "confirm('Sanity!')")
   end
 
   def test_function_with_href
-    assert_dom_equal %(<a href="http://example.com/" onclick="alert('Hello world!'); return false;">Greeting</a>),
+    assert_dom_equal %(<a href="http://example.com/" onclick="alert(&#x27;Hello world!&#x27;); return false;">Greeting</a>),
       link_to_function("Greeting", "alert('Hello world!')", :href => 'http://example.com/')
   end
 

diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb
@@ -77,7 +77,7 @@ def test_template_does_not_lose_its_source_after_rendering_if_it_does_not_have_a
   def test_locals
     @template = new_template("<%= my_local %>")
     @template.locals = [:my_local]
-    assert_equal "I'm a local", render(:my_local => "I'm a local")
+    assert_equal "I am a local", render(:my_local => "I am a local")
   end
 
   def test_restores_buffer

diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
@@ -193,7 +193,7 @@ def test_link_with_nil_html_options
 
   def test_link_tag_with_custom_onclick
     link = link_to("Hello", "http://www.example.com", :onclick => "alert('yay!')")
-    expected = %{<a href="http://www.example.com" onclick="alert('yay!')">Hello</a>}
+    expected = %{<a href="http://www.example.com" onclick="alert(&#x27;yay!&#x27;)">Hello</a>}
     assert_dom_equal expected, link
   end
 
@@ -203,12 +203,12 @@ def test_link_tag_with_javascript_confirm
       link_to("Hello", "http://www.example.com", :confirm => "Are you sure?")
     )
     assert_dom_equal(
-      "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure, can you?\">Hello</a>",
-      link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure, can you?")
+      "<a href=\"http://www.example.com\" data-confirm=\"You cant possibly be sure, can you?\">Hello</a>",
+      link_to("Hello", "http://www.example.com", :confirm => "You cant possibly be sure, can you?")
     )
     assert_dom_equal(
-      "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure,\n can you?\">Hello</a>",
-      link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure,\n can you?")
+      "<a href=\"http://www.example.com\" data-confirm=\"You cant possibly be sure,\n can you?\">Hello</a>",
+      link_to("Hello", "http://www.example.com", :confirm => "You cant possibly be sure,\n can you?")
     )
   end
 

diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -3,45 +3,24 @@
 
 class ERB
   module Util
-    HTML_ESCAPE = { '&' => '&amp;',  '>' => '&gt;',   '<' => '&lt;', '"' => '&quot;' }
+    HTML_ESCAPE = { '&' => '&amp;',  '>' => '&gt;',   '<' => '&lt;', '"' => '&quot;', "'" => '&#x27;' }
     JSON_ESCAPE = { '&' => '\u0026', '>' => '\u003E', '<' => '\u003C' }
 
-    # Detect whether 1.9 can transcode with XML escaping.
-    if '"&gt;&lt;&amp;&quot;"' == ('><&"'.encode('utf-8', :xml => :attr) rescue false)
-      # A utility method for escaping HTML tag characters.
-      # This method is also aliased as <tt>h</tt>.
-      #
-      # In your ERB templates, use this method to escape any unsafe content. For example:
-      #   <%=h @person.name %>
-      #
-      # ==== Example:
-      #   puts html_escape("is a > 0 & a < 10?")
-      #   # => is a &gt; 0 &amp; a &lt; 10?
-      def html_escape(s)
-        s = s.to_s
-        if s.html_safe?
-          s
-        else
-          s.encode(s.encoding, :xml => :attr)[1...-1].html_safe
-        end
-      end
-    else
-      # A utility method for escaping HTML tag characters.
-      # This method is also aliased as <tt>h</tt>.
-      #
-      # In your ERB templates, use this method to escape any unsafe content. For example:
-      #   <%=h @person.name %>
-      #
-      # ==== Example:
-      #   puts html_escape("is a > 0 & a < 10?")
-      #   # => is a &gt; 0 &amp; a &lt; 10?
-      def html_escape(s)
-        s = s.to_s
-        if s.html_safe?
-          s
-        else
-          s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }.html_safe
-        end
+    # A utility method for escaping HTML tag characters.
+    # This method is also aliased as <tt>h</tt>.
+    #
+    # In your ERB templates, use this method to escape any unsafe content. For example:
+    #   <%=h @person.name %>
+    #
+    # ==== Example:
+    #   puts html_escape("is a > 0 & a < 10?")
+    #   # => is a &gt; 0 &amp; a &lt; 10?
+    def html_escape(s)
+      s = s.to_s
+      if s.html_safe?
+        s
+      else
+        s.gsub(/[&"'><]/n) { |special| HTML_ESCAPE[special] }.html_safe
       end
     end
 

diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
@@ -464,8 +464,8 @@ def to_s
   end
 
   test "ERB::Util.html_escape should escape unsafe characters" do
-    string = '<>&"'
-    expected = '&lt;&gt;&amp;&quot;'
+    string = '<>&"\''
+    expected = '&lt;&gt;&amp;&quot;&#x27;'
     assert_equal expected, ERB::Util.html_escape(string)
   end
 

diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb
@@ -232,7 +232,7 @@ class ::PostsController < ActionController::Base ; end
 
       get '/posts'
       assert_match(/AssetNotPrecompiledError/, last_response.body)
-      assert_match(/app.js isn't precompiled/, last_response.body)
+      assert_match(/app.js isn&#x27;t precompiled/, last_response.body)
     end
 
     test "assets raise AssetNotPrecompiledError when manifest file is present and requested file isn't precompiled if digest is disabled" do
@@ -256,7 +256,7 @@ class ::PostsController < ActionController::Base ; end
 
       get '/posts'
       assert_match(/AssetNotPrecompiledError/, last_response.body)
-      assert_match(/app.js isn't precompiled/, last_response.body)
+      assert_match(/app.js isn&#x27;t precompiled/, last_response.body)
     end
 
     test "precompile properly refers files referenced with asset_path and and run in the provided RAILS_ENV" do