Attachment 863437 Details for Bug 1065520 – 4-0-number_helpers_xss.patch

[patch] 4-0-number_helpers_xss.patch

4-0-number_helpers_xss.patch (text/plain), 10.65 KB, created by Kurt Seifried on 2014-02-14 21:41:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kurt Seifried

Created: 2014-02-14 21:41:34 UTC

Size: 10.65 KB

patch

obsolete

>From 296d4e9861cdf0d5db2c3cc774e514d07feae395 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
> <rafaelmfranca@gmail.com>
>Date: Tue, 11 Feb 2014 23:22:58 -0200
>Subject: [PATCH] Escape format, negative_format and units options of number
> helpers
>
>Previously the values of these options were trusted leading to
>potential XSS vulnerabilities.
>---
> .../lib/action_view/helpers/number_helper.rb       | 29 ++++++++++------
> actionpack/test/template/number_helper_test.rb     | 39 ++++++++++++++++++++++
> 2 files changed, 58 insertions(+), 10 deletions(-)
>
>diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb
>index c6e2eaf..86d8468 100644
>--- a/actionpack/lib/action_view/helpers/number_helper.rb
>+++ b/actionpack/lib/action_view/helpers/number_helper.rb
>@@ -106,7 +106,7 @@ module ActionView
>       #   # => 1234567890,50 R$
>       def number_to_currency(number, options = {})
>         return unless number
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_currency(number, options)
>@@ -151,7 +151,7 @@ module ActionView
>       #   number_to_percentage("98a", raise: true)                         # => InvalidNumberError
>       def number_to_percentage(number, options = {})
>         return unless number
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_percentage(number, options)
>@@ -188,7 +188,7 @@ module ActionView
>       #
>       #  number_with_delimiter("112a", raise: true)              # => raise InvalidNumberError
>       def number_with_delimiter(number, options = {})
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_delimited(number, options)
>@@ -237,7 +237,7 @@ module ActionView
>       #   number_with_precision(1111.2345, precision: 2, separator: ',', delimiter: '.')
>       #   # => 1.111,23
>       def number_with_precision(number, options = {})
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_rounded(number, options)
>@@ -293,7 +293,7 @@ module ActionView
>       #   number_to_human_size(1234567890123, precision: 5)        # => "1.1229 TB"
>       #   number_to_human_size(524288000, precision: 5)            # => "500 MB"
>       def number_to_human_size(number, options = {})
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_human_size(number, options)
>@@ -399,7 +399,7 @@ module ActionView
>       #  number_to_human(0.34, units: :distance)                # => "34 centimeters"
>       #
>       def number_to_human(number, options = {})
>-        options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
>+        options = escape_unsafe_options(options.symbolize_keys)
> 
>         wrap_with_output_safety_handling(number, options.delete(:raise)) {
>           ActiveSupport::NumberHelper.number_to_human(number, options)
>@@ -408,13 +408,22 @@ module ActionView
> 
>       private
> 
>-      def escape_unsafe_delimiters_and_separators(options)
>-        options[:separator] = ERB::Util.html_escape(options[:separator]) if options[:separator] && !options[:separator].html_safe?
>-        options[:delimiter] = ERB::Util.html_escape(options[:delimiter]) if options[:delimiter] && !options[:delimiter].html_safe?
>-        options[:unit]      = ERB::Util.html_escape(options[:unit]) if options[:unit] && !options[:unit].html_safe?
>+      def escape_unsafe_options(options)
>+        options[:format]          = ERB::Util.html_escape(options[:format]) if options[:format]
>+        options[:negative_format] = ERB::Util.html_escape(options[:negative_format]) if options[:negative_format]
>+        options[:separator]       = ERB::Util.html_escape(options[:separator]) if options[:separator]
>+        options[:delimiter]       = ERB::Util.html_escape(options[:delimiter]) if options[:delimiter]
>+        options[:unit]            = ERB::Util.html_escape(options[:unit]) if options[:unit] && !options[:unit].html_safe?
>+        options[:units]           = escape_units(options[:units]) if options[:units] && Hash === options[:units]
>         options
>       end
> 
>+      def escape_units(units)
>+        Hash[units.map do |k, v|
>+          [k, ERB::Util.html_escape(v)]
>+        end]
>+      end
>+
>       def wrap_with_output_safety_handling(number, raise_on_invalid, &block)
>         valid_float = valid_float?(number)
>         raise InvalidNumberError, number if raise_on_invalid && !valid_float
>diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb
>index be336ea..11bc978 100644
>--- a/actionpack/test/template/number_helper_test.rb
>+++ b/actionpack/test/template/number_helper_test.rb
>@@ -8,6 +8,8 @@ class NumberHelperTest < ActionView::TestCase
>     assert_equal "555-1234", number_to_phone(5551234)
>     assert_equal "(800) 555-1212 x 123", number_to_phone(8005551212, area_code: true, extension: 123)
>     assert_equal "+18005551212", number_to_phone(8005551212, country_code: 1, delimiter: "")
>+    assert_equal "+&lt;script&gt;&lt;/script&gt;8005551212", number_to_phone(8005551212, country_code: "<script></script>", delimiter: "")
>+    assert_equal "8005551212 x &lt;script&gt;&lt;/script&gt;", number_to_phone(8005551212, extension: "<script></script>", delimiter: "")
>   end
> 
>   def test_number_to_currency
>@@ -16,11 +18,17 @@ class NumberHelperTest < ActionView::TestCase
>     assert_equal "$1,234,567,892", number_to_currency(1234567891.50, precision: 0)
>     assert_equal "1,234,567,890.50 - K&#269;", number_to_currency("-1234567890.50", unit: raw("K&#269;"), format: "%n %u", negative_format: "%n - %u")
>     assert_equal "&amp;pound;1,234,567,890.50", number_to_currency("1234567890.50", unit: "&pound;")
>+    assert_equal "&lt;b&gt;1,234,567,890.50&lt;/b&gt; $", number_to_currency("1234567890.50", format: "<b>%n</b> %u")
>+    assert_equal "&lt;b&gt;1,234,567,890.50&lt;/b&gt; $", number_to_currency("-1234567890.50", negative_format: "<b>%n</b> %u")
>+    assert_equal "&lt;b&gt;1,234,567,890.50&lt;/b&gt; $", number_to_currency("-1234567890.50", 'negative_format' => "<b>%n</b> %u")
>   end
> 
>   def test_number_to_percentage
>     assert_equal nil, number_to_percentage(nil)
>     assert_equal "100.000%", number_to_percentage(100)
>+    assert_equal "100.000 %", number_to_percentage(100, format: '%n %')
>+    assert_equal "&lt;b&gt;100.000&lt;/b&gt; %", number_to_percentage(100, format: '<b>%n</b> %')
>+    assert_equal "<b>100.000</b> %", number_to_percentage(100, format: raw('<b>%n</b> %'))
>     assert_equal "100%", number_to_percentage(100, precision: 0)
>     assert_equal "123.4%", number_to_percentage(123.400, precision: 3, strip_insignificant_zeros: true)
>     assert_equal "1.000,000%", number_to_percentage(1000, delimiter: ".", separator: ",")
>@@ -52,6 +60,31 @@ class NumberHelperTest < ActionView::TestCase
>     assert_equal "489.0 Thousand", number_to_human(489000, precision: 4, strip_insignificant_zeros: false)
>   end
> 
>+  def test_number_to_human_escape_units
>+    volume = { unit: "<b>ml</b>", thousand: "<b>lt</b>", million: "<b>m3</b>", trillion: "<b>km3</b>", quadrillion: "<b>Pl</b>" }
>+    assert_equal '123 &lt;b&gt;lt&lt;/b&gt;', number_to_human(123456, :units => volume)
>+    assert_equal '12 &lt;b&gt;ml&lt;/b&gt;', number_to_human(12, :units => volume)
>+    assert_equal '1.23 &lt;b&gt;m3&lt;/b&gt;', number_to_human(1234567, :units => volume)
>+    assert_equal '1.23 &lt;b&gt;km3&lt;/b&gt;', number_to_human(1_234_567_000_000, :units => volume)
>+    assert_equal '1.23 &lt;b&gt;Pl&lt;/b&gt;', number_to_human(1_234_567_000_000_000, :units => volume)
>+
>+    #Including fractionals
>+    distance = { mili: "<b>mm</b>", centi: "<b>cm</b>", deci: "<b>dm</b>", unit: "<b>m</b>",
>+                 ten: "<b>dam</b>", hundred: "<b>hm</b>", thousand: "<b>km</b>",
>+                 micro: "<b>um</b>", nano: "<b>nm</b>", pico: "<b>pm</b>", femto: "<b>fm</b>"}
>+    assert_equal '1.23 &lt;b&gt;mm&lt;/b&gt;', number_to_human(0.00123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;cm&lt;/b&gt;', number_to_human(0.0123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;dm&lt;/b&gt;', number_to_human(0.123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;m&lt;/b&gt;', number_to_human(1.23, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;dam&lt;/b&gt;', number_to_human(12.3, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;hm&lt;/b&gt;', number_to_human(123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;km&lt;/b&gt;', number_to_human(1230, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;um&lt;/b&gt;', number_to_human(0.00000123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;nm&lt;/b&gt;', number_to_human(0.00000000123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;pm&lt;/b&gt;', number_to_human(0.00000000000123, :units => distance)
>+    assert_equal '1.23 &lt;b&gt;fm&lt;/b&gt;', number_to_human(0.00000000000000123, :units => distance)
>+  end
>+
>   def test_number_helpers_escape_delimiter_and_separator
>     assert_equal "111&lt;script&gt;&lt;/script&gt;111&lt;script&gt;&lt;/script&gt;1111", number_to_phone(1111111111, delimiter: "<script></script>")
> 
>@@ -73,6 +106,12 @@ class NumberHelperTest < ActionView::TestCase
>     assert_equal "100&lt;script&gt;&lt;/script&gt;000 Quadrillion", number_to_human(10**20, delimiter: "<script></script>")
>   end
> 
>+  def test_number_to_human_with_custom_translation_scope
>+    I18n.backend.store_translations 'ts',
>+      :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"}
>+    assert_equal "1.01 cm", number_to_human(0.0101, :locale => 'ts', :units => :custom_units_for_number_to_human)
>+  end
>+
>   def test_number_helpers_outputs_are_html_safe
>     assert number_to_human(1).html_safe?
>     assert !number_to_human("<script></script>").html_safe?
>-- 
>1.8.4.3
>

Actions: View | Diff

Attachments on bug 1065520: 863436 | 863437 | 863438