[CVE-2012-3464] 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 rails#7215
Tho85 · Mar 27, 2013 · da90a25 · da90a25
1 parent 0b9ddf1
commit da90a25
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 61 deletions.
diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb
@@ -126,7 +126,7 @@ class TestWithoutLayout < Rack::TestCase
     test "rendering a template with error properly excerts 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
@@ -170,7 +170,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
 
@@ -823,7 +823,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
@@ -444,14 +444,14 @@ def test_image_tag
   def test_image_tag_with_mouseover
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
       assert_dom_equal(
-        %(<img alt="Mouse" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" src="/images/mouse.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 => "/images/mouse_over.png")
       )
     end
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
       assert_dom_equal(
-        %(<img alt="Mouse" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" src="/images/mouse.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"))
       )
     end
@@ -1113,11 +1113,11 @@ def test_should_compute_proper_path
     assert_dom_equal(%(/collaboration/hieraki/images/xml.png), image_path("xml.png"))
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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"))
     end
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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
   end
 
@@ -1133,11 +1133,11 @@ def test_should_compute_proper_path_with_asset_host
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png"))
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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"))
     end
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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
   end
 
@@ -1149,11 +1149,11 @@ def test_should_compute_proper_path_with_asset_host_and_default_protocol
     assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png"))
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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"))
     end
 
     assert_deprecated ":mouseover option is deprecated and will be removed from Rails 4.0" do
-      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="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
   end
 

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
@@ -1011,7 +1011,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=\"" + ERB::Util.html_escape("alert('Hello World');") + "\">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
@@ -1047,13 +1047,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
@@ -369,7 +369,7 @@ def test_stringify_symbol_keys
   def test_submit_tag
     assert_deprecated ":disable_with option is deprecated and will be removed from Rails 4.0. Use 'data-disable-with' instead" do
       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=") + ERB::Util.html_escape("alert('hello!')") + %(" 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
@@ -47,14 +47,14 @@ def test_escape_javascript_with_safebuffer
 
   def test_button_to_function
     assert_deprecated "button_to_function is deprecated and will be removed from Rails 4.0" do
-      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
   end
 
   def test_button_to_function_with_onclick
     assert_deprecated "button_to_function is deprecated and will be removed from Rails 4.0" do
-      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
   end
@@ -68,21 +68,21 @@ def test_button_to_function_without_function
 
   def test_link_to_function
     assert_deprecated "link_to_function is deprecated and will be removed from Rails 4.0" do
-      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
   end
 
   def test_link_to_function_with_existing_onclick
     assert_deprecated "link_to_function is deprecated and will be removed from Rails 4.0" do
-      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
   end
 
   def test_function_with_href
     assert_deprecated "link_to_function is deprecated and will be removed from Rails 4.0" do
-      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
   end

diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb
@@ -79,7 +79,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
@@ -201,7 +201,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="} + ERB::Util.html_escape("alert('yay!')") + %{">Hello</a>}
     assert_dom_equal expected, link
   end
 
@@ -211,12 +211,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,36 +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
-      def html_escape(s) #:nodoc:
-        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
@@ -493,8 +493,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