Skip to content

Commit

Permalink
backport fix for CVE-2014-0081 (XSS vulnerability in number_to_currency)
Browse files Browse the repository at this point in the history
Note that number_to_percentage is not affected, number_to_human does not
exist, and only the :format parameter to number_to_currency is not
escaped.
  • Loading branch information
kraatob committed Feb 19, 2014
1 parent 4f4fa7c commit 8579261
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GEM
nokogiri (1.3.3)
pg (0.9.0)
rack (1.1.0)
railslts-version (2.3.18.6)
railslts-version (2.3.18.8)
rake (0.8.7)
rdoc (2.5.11)
sqlite3 (1.3.7)
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_view/helpers/number_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def number_to_currency(number, options = {})
unit = options[:unit] || defaults[:unit]
separator = options[:separator] || defaults[:separator]
delimiter = options[:delimiter] || defaults[:delimiter]
format = options[:format] || defaults[:format]
format = options[:format] ? ERB::Util.html_escape(options[:format]) : defaults[:format]
separator = '' if precision == 0

begin
Expand Down
25 changes: 25 additions & 0 deletions actionpack/test/template/number_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,28 @@
class NumberHelperTest < ActionView::TestCase
tests ActionView::Helpers::NumberHelper


def test_number_helpers_escape_delimiter_and_separator
# html-escaping the right side is fine, since we only have a problem if the helper does
# not escape AND return a safe string

assert_equal "111&lt;script&gt;&lt;/script&gt;111&lt;script&gt;&lt;/script&gt;1111", ERB::Util.html_escape(number_to_phone(1111111111, :delimiter => "<script></script>"))

assert_equal "$1&lt;script&gt;&lt;/script&gt;01", ERB::Util.html_escape(number_to_currency(1.01, :separator => "<script></script>"))
assert_equal "$1&lt;script&gt;&lt;/script&gt;000.00", ERB::Util.html_escape(number_to_currency(1000, :delimiter => "<script></script>"))

assert_equal "1&lt;script&gt;&lt;/script&gt;010%", ERB::Util.html_escape(number_to_percentage(1.01, :separator => "<script></script>"))
assert_equal "1&lt;script&gt;&lt;/script&gt;000.000%", ERB::Util.html_escape(number_to_percentage(1000, :delimiter => "<script></script>"))

assert_equal "1&lt;script&gt;&lt;/script&gt;01", ERB::Util.html_escape(number_with_delimiter(1.01, :separator => "<script></script>"))
assert_equal "1&lt;script&gt;&lt;/script&gt;000", ERB::Util.html_escape(number_with_delimiter(1000, :delimiter => "<script></script>"))

assert_equal "1&lt;script&gt;&lt;/script&gt;010", ERB::Util.html_escape(number_with_precision(1.01, :separator => "<script></script>"))
assert_equal "1&lt;script&gt;&lt;/script&gt;000.000", ERB::Util.html_escape(number_with_precision(1000, :delimiter => "<script></script>"))

assert_equal "9&lt;script&gt;&lt;/script&gt;9 KB", ERB::Util.html_escape(number_to_human_size(10100, :separator => "<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 @@ -15,6 +37,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", ERB::Util.html_escape(number_to_phone(8005551212, :country_code => "<script></script>", :delimiter => ""))
assert_equal "8005551212 x &lt;script&gt;&lt;/script&gt;", ERB::Util.html_escape(number_to_phone(8005551212, :extension => "<script></script>", :delimiter => ""))
assert_equal("x", number_to_phone("x"))
assert_nil number_to_phone(nil)
end
Expand All @@ -28,6 +52,7 @@ def test_number_to_currency
assert_equal("&amp;pound;1234567890,50", number_to_currency(1234567890.50, {:unit => "&pound;", :separator => ",", :delimiter => ""}))
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 "&lt;b&gt;1,234,567,890.50&lt;/b&gt; $", ERB::Util.html_escape(number_to_currency("1234567890.50", :format => "<b>%n</b> %u"))
#assert_equal("$x.", number_to_currency("x")) # fails due to API consolidation
assert_equal("$x", number_to_currency("x"))
assert_nil number_to_currency(nil)
Expand Down
2 changes: 1 addition & 1 deletion railties/railties.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ Gem::Specification.new do |s|
s.add_dependency 'actionpack', '= 2.3.18'
s.add_dependency 'actionmailer', '= 2.3.18'
s.add_dependency 'activeresource', '= 2.3.18'
s.add_dependency 'railslts-version', '= 2.3.18.7'
s.add_dependency 'railslts-version', '= 2.3.18.8'
end

0 comments on commit 8579261

Please sign in to comment.