Escape format, negative_format and units options of number helpers

Previously the values of these options were trusted leading to
potential XSS vulnerabilities.

Fixes: CVE-2014-0081

2 files changed, 53 insertions, 5 deletions

diff --git a/actionview/lib/action_view/helpers/number_helper.rb b/actionview/lib/action_view/helpers/number_helper.rb
index ad825cd1f1..7157a95146 100644
--- a/actionview/lib/action_view/helpers/number_helper.rb
+++ b/actionview/lib/action_view/helpers/number_helper.rb
@@ -384,20 +384,29 @@ module ActionView
 
       def delegate_number_helper_method(method, 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.public_send(method, number, options)
         }
       end
 
-      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/actionview/test/template/number_helper_test.rb b/actionview/test/template/number_helper_test.rb
index be336ea3fb..11bc978324 100644
--- a/actionview/test/template/number_helper_test.rb
+++ b/actionview/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?