Skip to content

Commit

Permalink
Escape format, negative_format and units options of number helpers
Browse files Browse the repository at this point in the history 
Previously the values of these options were trusted leading to
potential XSS vulnerabilities.

Fixes: CVE-2014-0081
  • Loading branch information
@rafaelfranca
rafaelfranca committed Feb 18, 2014
1 parent 6422630 commit eaa2101
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 1 deletion.
14 changes: 13 additions & 1 deletion actionpack/lib/action_view/helpers/number_helper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,18 @@ def number_to_currency(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]
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)
Expand Down Expand Up @@ -206,6 +212,9 @@ def number_to_percentage(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]

defaults = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
percentage = I18n.translate(:'number.percentage.format', :locale => options[:locale], :default => {})
defaults = defaults.merge(percentage)
Expand Down Expand Up @@ -255,6 +264,9 @@ def number_to_percentage(number, options = {})
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
Expand Down Expand Up @@ -578,7 +590,7 @@ def number_to_human(number, options = {})
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
Expand Down
51 changes: 51 additions & 0 deletions actionpack/test/template/number_helper_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,27 @@ def terabytes(number)
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))
Expand All @@ -33,6 +54,8 @@ def test_number_to_phone
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
Expand All @@ -48,6 +71,9 @@ def test_number_to_currency
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
Expand Down Expand Up @@ -252,6 +278,31 @@ def test_number_to_human_with_custom_units
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 => {})
Expand Down

0 comments on commit eaa2101

Please sign in to comment.