From 9e2d63d51a5e61bf1b0e7e369805aad84956cbe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= 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 £ 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 "+<script></script>8005551212", number_to_phone(8005551212, country_code: "", delimiter: "") + assert_equal "8005551212 x <script></script>", number_to_phone(8005551212, extension: "", 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č", number_to_currency("-1234567890.50", unit: raw("Kč"), format: "%n %u", negative_format: "%n - %u") assert_equal "&pound;1,234,567,890.50", number_to_currency("1234567890.50", unit: "£") + assert_equal "<b>1,234,567,890.50</b> $", number_to_currency("1234567890.50", format: "%n %u") + assert_equal "<b>1,234,567,890.50</b> $", number_to_currency("-1234567890.50", negative_format: "%n %u") + assert_equal "<b>1,234,567,890.50</b> $", number_to_currency("-1234567890.50", 'negative_format' => "%n %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 "<b>100.000</b> %", number_to_percentage(100, format: '%n %') + assert_equal "100.000 %", number_to_percentage(100, format: raw('%n %')) 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: "ml", thousand: "lt", million: "m3", trillion: "km3", quadrillion: "Pl" } + assert_equal '123 <b>lt</b>', number_to_human(123456, :units => volume) + assert_equal '12 <b>ml</b>', number_to_human(12, :units => volume) + assert_equal '1.23 <b>m3</b>', number_to_human(1234567, :units => volume) + assert_equal '1.23 <b>km3</b>', number_to_human(1_234_567_000_000, :units => volume) + assert_equal '1.23 <b>Pl</b>', number_to_human(1_234_567_000_000_000, :units => volume) + + #Including fractionals + distance = { mili: "mm", centi: "cm", deci: "dm", unit: "m", + ten: "dam", hundred: "hm", thousand: "km", + micro: "um", nano: "nm", pico: "pm", femto: "fm"} + assert_equal '1.23 <b>mm</b>', number_to_human(0.00123, :units => distance) + assert_equal '1.23 <b>cm</b>', number_to_human(0.0123, :units => distance) + assert_equal '1.23 <b>dm</b>', number_to_human(0.123, :units => distance) + assert_equal '1.23 <b>m</b>', number_to_human(1.23, :units => distance) + assert_equal '1.23 <b>dam</b>', number_to_human(12.3, :units => distance) + assert_equal '1.23 <b>hm</b>', number_to_human(123, :units => distance) + assert_equal '1.23 <b>km</b>', number_to_human(1230, :units => distance) + assert_equal '1.23 <b>um</b>', number_to_human(0.00000123, :units => distance) + assert_equal '1.23 <b>nm</b>', number_to_human(0.00000000123, :units => distance) + assert_equal '1.23 <b>pm</b>', number_to_human(0.00000000000123, :units => distance) + assert_equal '1.23 <b>fm</b>', number_to_human(0.00000000000000123, :units => distance) + end + def test_number_helpers_escape_delimiter_and_separator assert_equal "111<script></script>111<script></script>1111", number_to_phone(1111111111, delimiter: "") @@ -73,6 +106,12 @@ class NumberHelperTest < ActionView::TestCase assert_equal "100<script></script>000 Quadrillion", number_to_human(10**20, delimiter: "") 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("").html_safe? -- 1.8.4.3