Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Merge branch '3-1-stable-security' into 3-1-4

* 3-1-stable-security:
  Ensure [] respects the status of the buffer.
  use AS::SafeBuffer#clone_empty for flushing the output_buffer
  add AS::SafeBuffer#clone_empty
  fix output safety issue with select options
  • Loading branch information...
commit d1fc35fe196ee2913bc34e5c581a5284610d05d1 2 parents 8c677e9 + 3d86727
Aaron Patterson tenderlove authored
2  actionpack/lib/action_view/helpers/capture_helper.rb
@@ -194,7 +194,7 @@ def with_output_buffer(buf = nil) #:nodoc:
194 194 def flush_output_buffer #:nodoc:
195 195 if output_buffer && !output_buffer.empty?
196 196 response.body_parts << output_buffer
197   - self.output_buffer = output_buffer[0,0]
  197 + self.output_buffer = output_buffer.respond_to?(:clone_empty) ? output_buffer.clone_empty : output_buffer[0, 0]
198 198 nil
199 199 end
200 200 end
6 actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -595,13 +595,13 @@ def to_time_zone_select_tag(priority_zones, options, html_options)
595 595 private
596 596 def add_options(option_tags, options, value = nil)
597 597 if options[:include_blank]
598   - option_tags = "<option value=\"\">#{ERB::Util.html_escape(options[:include_blank]) if options[:include_blank].kind_of?(String)}</option>\n" + option_tags
  598 + option_tags = content_tag('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags
599 599 end
600 600 if value.blank? && options[:prompt]
601 601 prompt = options[:prompt].kind_of?(String) ? options[:prompt] : I18n.translate('helpers.select.prompt', :default => 'Please select')
602   - option_tags = "<option value=\"\">#{ERB::Util.html_escape(prompt)}</option>\n" + option_tags
  602 + option_tags = content_tag('option', prompt, :value => '') + "\n" + option_tags
603 603 end
604   - option_tags.html_safe
  604 + option_tags
605 605 end
606 606 end
607 607
9 actionpack/test/template/form_options_helper_test.rb
@@ -452,7 +452,7 @@ def @post.to_param; 108; end
452 452
453 453 def test_select_under_fields_for_with_string_and_given_prompt
454 454 @post = Post.new
455   - options = "<option value=\"abe\">abe</option><option value=\"mus\">mus</option><option value=\"hest\">hest</option>"
  455 + options = "<option value=\"abe\">abe</option><option value=\"mus\">mus</option><option value=\"hest\">hest</option>".html_safe
456 456
457 457 output_buffer = fields_for :post, @post do |f|
458 458 concat f.select(:category, options, :prompt => 'The prompt')
@@ -556,6 +556,13 @@ def test_select_with_index_option
556 556 )
557 557 end
558 558
  559 + def test_select_escapes_options
  560 + assert_dom_equal(
  561 + '<select id="post_title" name="post[title]">&lt;script&gt;alert(1)&lt;/script&gt;</select>',
  562 + select('post', 'title', '<script>alert(1)</script>')
  563 + )
  564 + end
  565 +
559 566 def test_select_with_selected_nil
560 567 @post = Post.new
561 568 @post.category = "<mus>"
50 activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -86,23 +86,41 @@ def initialize
86 86 end
87 87 end
88 88
  89 + def [](*args)
  90 + return super if args.size < 2
  91 +
  92 + if html_safe?
  93 + new_safe_buffer = super
  94 + new_safe_buffer.instance_eval { @html_safe = true }
  95 + new_safe_buffer
  96 + else
  97 + to_str[*args]
  98 + end
  99 + end
  100 +
89 101 def safe_concat(value)
90   - raise SafeConcatError if dirty?
  102 + raise SafeConcatError unless html_safe?
91 103 original_concat(value)
92 104 end
93 105
94 106 def initialize(*)
95   - @dirty = false
  107 + @html_safe = true
96 108 super
97 109 end
98 110
99 111 def initialize_copy(other)
100 112 super
101   - @dirty = other.dirty?
  113 + @html_safe = other.html_safe?
  114 + end
  115 +
  116 + def clone_empty
  117 + new_safe_buffer = self[0, 0]
  118 + new_safe_buffer.instance_variable_set(:@dirty, @dirty)
  119 + new_safe_buffer
102 120 end
103 121
104 122 def concat(value)
105   - if dirty? || value.html_safe?
  123 + if !html_safe? || value.html_safe?
106 124 super(value)
107 125 else
108 126 super(ERB::Util.h(value))
@@ -115,7 +133,7 @@ def +(other)
115 133 end
116 134
117 135 def html_safe?
118   - !dirty?
  136 + defined?(@html_safe) && @html_safe
119 137 end
120 138
121 139 def to_s
@@ -138,23 +156,17 @@ def to_yaml(*args)
138 156 for unsafe_method in UNSAFE_STRING_METHODS
139 157 if 'String'.respond_to?(unsafe_method)
140 158 class_eval <<-EOT, __FILE__, __LINE__ + 1
141   - def #{unsafe_method}(*args)
142   - super.to_str
143   - end
144   -
145   - def #{unsafe_method}!(*args)
146   - @dirty = true
147   - super
148   - end
  159 + def #{unsafe_method}(*args, &block) # def capitalize(*args, &block)
  160 + to_str.#{unsafe_method}(*args, &block) # to_str.capitalize(*args, &block)
  161 + end # end
  162 +
  163 + def #{unsafe_method}!(*args) # def capitalize!(*args)
  164 + @html_safe = false # @html_safe = false
  165 + super # super
  166 + end # end
149 167 EOT
150 168 end
151 169 end
152   -
153   - protected
154   -
155   - def dirty?
156   - @dirty
157   - end
158 170 end
159 171 end
160 172
45 activesupport/test/safe_buffer_test.rb
@@ -85,21 +85,60 @@ def setup
85 85 assert_equal "hello&lt;&gt;", clean + @buffer
86 86 end
87 87
88   - test "Should concat as a normal string when dirty" do
  88 + test "Should concat as a normal string when safe" do
89 89 clean = "hello".html_safe
90 90 @buffer.gsub!('', '<>')
91 91 assert_equal "<>hello", @buffer + clean
92 92 end
93 93
94   - test "Should preserve dirty? status on copy" do
  94 + test "Should preserve html_safe? status on copy" do
95 95 @buffer.gsub!('', '<>')
96 96 assert !@buffer.dup.html_safe?
97 97 end
98 98
99   - test "Should raise an error when safe_concat is called on dirty buffers" do
  99 + test "Should return safe buffer when added with another safe buffer" do
  100 + clean = "<script>".html_safe
  101 + result_buffer = @buffer + clean
  102 + assert result_buffer.html_safe?
  103 + assert_equal "<script>", result_buffer
  104 + end
  105 +
  106 + test "Should raise an error when safe_concat is called on unsafe buffers" do
100 107 @buffer.gsub!('', '<>')
101 108 assert_raise ActiveSupport::SafeBuffer::SafeConcatError do
102 109 @buffer.safe_concat "BUSTED"
103 110 end
104 111 end
  112 +
  113 + test "Should not fail if the returned object is not a string" do
  114 + assert_kind_of NilClass, @buffer.slice("chipchop")
  115 + end
  116 +
  117 + test "clone_empty returns an empty buffer" do
  118 + assert_equal '', ActiveSupport::SafeBuffer.new('foo').clone_empty
  119 + end
  120 +
  121 + test "clone_empty keeps the original dirtyness" do
  122 + assert @buffer.clone_empty.html_safe?
  123 + assert !@buffer.gsub!('', '').clone_empty.html_safe?
  124 + end
  125 +
  126 + test "Should be safe when sliced if original value was safe" do
  127 + new_buffer = @buffer[0,0]
  128 + assert_not_nil new_buffer
  129 + assert new_buffer.html_safe?, "should be safe"
  130 + end
  131 +
  132 + test "Should continue unsafe on slice" do
  133 + x = 'foo'.html_safe.gsub!('f', '<script>alert("lolpwnd");</script>')
  134 +
  135 + # calling gsub! makes the dirty flag true
  136 + assert !x.html_safe?, "should not be safe"
  137 +
  138 + # getting a slice of it
  139 + y = x[0..-1]
  140 +
  141 + # should still be unsafe
  142 + assert !y.html_safe?, "should not be safe"
  143 + end
105 144 end

0 comments on commit d1fc35f

Please sign in to comment.
Something went wrong with that request. Please try again.