Attachment 863436 Details for Bug 1065520 – 3-2-number_helpers_xss.patch

[patch] 3-2-number_helpers_xss.patch

3-2-number_helpers_xss.patch (text/plain), 8.92 KB, created by Kurt Seifried on 2014-02-14 21:41:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kurt Seifried

Created: 2014-02-14 21:41:13 UTC

Size: 8.92 KB

patch

obsolete

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

Actions: View | Diff

Attachments on bug 1065520: 863436 | 863437 | 863438