Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Tue, 18 Feb 2014 11:03:09 -0800
From: Aaron Patterson <tenderlove@...y-lang.org>
To: rubyonrails-security@...glegroups.com, oss-security@...ts.openwall.com,
	secalert@...hat.com
Subject: XSS Vulnerability in number_to_currency, number_to_percentage and
 number_to_human (CVE-2014-0081)

XSS Vulnerability in number_to_currency, number_to_percentage and number_to_human

There is an XSS vulnerability in the number_to_currency, number_to_percentage
and number_to_human helpers in Ruby on Rails. This vulnerability has been
assigned the CVE identifier CVE-2014-0081.

Versions Affected:  All.
Fixed Versions:     4.1.0.beta2, 4.0.3, 3.2.17.

Impact
------
These helpers allows users to nicely format a numeric value. Some of the parameters
to the helper (format, negative_format and units) are not escaped correctly.
Application which pass user controlled data as one of these parameters are
vulnerable to an XSS attack.

All users passing user controlled data to these parameters of the number helpers
should either upgrade or use one of the workarounds immediately.

Releases
--------
The 4.1.0.rc1, 4.0.3 and 3.2.17 releases are available at the normal locations.

Workarounds
-----------

The workaround for this issue is to escape the value passed to the parameter.
For example, replace code like this:

```
<%= number_to_currency(1.02, format: params[:format]) %>
```

With code like this

```
<%= number_to_currency(1.02, format: h(params[:format])) %>
```

Patches
-------
To aid users who aren't able to upgrade immediately we have provided patches for
the two supported release series. They are in git-am format and consist of a
single changeset.

* 4-1-beta-number_helpers_xss.patch - Patch for 4.1-beta series
* 4-0-number_helpers_xss.patch - Patch for 4.0 series
* 3-2-number_helpers_xss.patch - Patch for 3.2 series

Please note that only the 4.0.x and 3.2.x series are supported at present. Users
of earlier unsupported releases are advised to upgrade as soon as possible as we
cannot guarantee the continued availability of security fixes for unsupported
releases.

Credits
-------

Thanks to Kevin Reintjes for reporting the issue to us.

-- 
Aaron Patterson
http://tenderlovemaking.com/

From af9cac1d311f6564a2927c23f42e7194e4a189ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@...il.com>
Date: Tue, 11 Feb 2014 23:29:27 -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.

Fixes: CVE-2014-0081
---
 .../lib/action_view/helpers/number_helper.rb       | 14 +++++-
 actionpack/test/template/number_helper_test.rb     | 51 ++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb
index 2e04ff4..8ebd7e2 100644
--- a/actionpack/lib/action_view/helpers/number_helper.rb
+++ b/actionpack/lib/action_view/helpers/number_helper.rb
@@ -138,12 +138,18 @@ module ActionView
 
         options.symbolize_keys!
 
+        options[:delimiter] = ERB::Util.html_escape(options[:delimiter]) if options[:delimiter]
+        options[:separator] = ERB::Util.html_escape(options[:separator]) if options[:separator]
+        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]
+
         defaults  = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
         currency  = I18n.translate(:'number.currency.format', :locale => options[:locale], :default => {})
         currency[:negative_format] ||= "-" + currency[:format] if currency[:format]
 
         defaults  = DEFAULT_CURRENCY_VALUES.merge(defaults).merge!(currency)
         defaults[:negative_format] = "-" + options[:format] if options[:format]
+
         options   = defaults.merge!(options)
 
         unit      = options.delete(:unit)
@@ -206,6 +212,9 @@ module ActionView
 
         options.symbolize_keys!
 
+        options[:delimiter] = ERB::Util.html_escape(options[:delimiter]) if options[:delimiter]
+        options[:separator] = ERB::Util.html_escape(options[:separator]) if options[:separator]
+
         defaults   = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
         percentage = I18n.translate(:'number.percentage.format', :locale => options[:locale], :default => {})
         defaults  = defaults.merge(percentage)
@@ -255,6 +264,9 @@ module ActionView
       def number_with_delimiter(number, options = {})
         options.symbolize_keys!
 
+        options[:delimiter] = ERB::Util.html_escape(options[:delimiter]) if options[:delimiter]
+        options[:separator] = ERB::Util.html_escape(options[:separator]) if options[:separator]
+
         begin
           Float(number)
         rescue ArgumentError, TypeError
@@ -578,7 +590,7 @@ module ActionView
         units = options.delete :units
         unit_exponents = case units
         when Hash
-          units
+          units = Hash[units.map { |k, v| [k, ERB::Util.html_escape(v)] }]
         when String, Symbol
           I18n.translate(:"#{units}", :locale => options[:locale], :raise => true)
         when nil
diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb
index 22da7e2..7f78b52 100644
--- a/actionpack/test/template/number_helper_test.rb
+++ b/actionpack/test/template/number_helper_test.rb
@@ -19,6 +19,27 @@ class NumberHelperTest < ActionView::TestCase
     gigabytes(number) * 1024
   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>")
+
+    assert_equal "$1&lt;script&gt;&lt;/script&gt;01", number_to_currency(1.01, :separator => "<script></script>")
+    assert_equal "$1&lt;script&gt;&lt;/script&gt;000.00", number_to_currency(1000, :delimiter => "<script></script>")
+
+    assert_equal "1&lt;script&gt;&lt;/script&gt;010%", number_to_percentage(1.01, :separator => "<script></script>")
+    assert_equal "1&lt;script&gt;&lt;/script&gt;000.000%", number_to_percentage(1000, :delimiter => "<script></script>")
+
+    assert_equal "1&lt;script&gt;&lt;/script&gt;01", number_with_delimiter(1.01, :separator => "<script></script>")
+    assert_equal "1&lt;script&gt;&lt;/script&gt;000", number_with_delimiter(1000, :delimiter => "<script></script>")
+
+    assert_equal "1&lt;script&gt;&lt;/script&gt;010", number_with_precision(1.01, :separator => "<script></script>")
+    assert_equal "1&lt;script&gt;&lt;/script&gt;000.000", number_with_precision(1000, :delimiter => "<script></script>")
+
+    assert_equal "9&lt;script&gt;&lt;/script&gt;86 KB", number_to_human_size(10100, :separator => "<script></script>")
+
+    assert_equal "1&lt;script&gt;&lt;/script&gt;01", number_to_human(1.01, :separator => "<script></script>")
+    assert_equal "100&lt;script&gt;&lt;/script&gt;000 Quadrillion", number_to_human(10**20, :delimiter => "<script></script>")
+  end
+
   def test_number_to_phone
     assert_equal("555-1234", number_to_phone(5551234))
     assert_equal("800-555-1212", number_to_phone(8005551212))
@@ -33,6 +54,8 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("+18005551212", number_to_phone(8005551212, :country_code => 1, :delimiter => ''))
     assert_equal("22-555-1212", number_to_phone(225551212))
     assert_equal("+45-22-555-1212", number_to_phone(225551212, :country_code => 45))
+    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
@@ -48,6 +71,9 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("$1,234,567,890.50", number_to_currency("1234567890.50"))
     assert_equal("1,234,567,890.50 K&#269;", number_to_currency("1234567890.50", {:unit => raw("K&#269;"), :format => "%n %u"}))
     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 "&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
@@ -252,6 +278,31 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal '4.5  tens', number_to_human(45, :units => {:unit => "", :ten => ' tens   '})
   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_to_human_with_custom_units_that_are_missing_the_needed_key
     assert_equal '123', number_to_human(123, :units => {:thousand => 'k'})
     assert_equal '123', number_to_human(123, :units => {})
-- 
1.8.4.3


From 9e2d63d51a5e61bf1b0e7e369805aad84956cbe2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@...il.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.

Fixes: CVE-2014-0081
---
 .../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 f3914e4..1cb0e7e 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 &pound;
       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


From 82dc2cdd48a900c2479fe091ad02bb30beeeb885 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
 <rafaelmfranca@...il.com>
Date: Tue, 11 Feb 2014 23:36:10 -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.

Fixes: CVE-2014-0081
---
 .../lib/action_view/helpers/number_helper.rb       | 19 ++++++++---
 actionview/test/template/number_helper_test.rb     | 39 ++++++++++++++++++++++
 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 ad825cd..7157a95 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 be336ea..11bc978 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?
-- 
1.8.4.3



[ CONTENT OF TYPE application/pgp-signature SKIPPED ]

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ