Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

We’re showing branches in this repository, but you can also compare across forks.

base fork: rails/rails
...
head fork: rails/rails
  • 12 commits
  • 33 files changed
  • 4 commit comments
  • 9 contributors
Commits on Feb 12, 2013
Carlos Antonio da Silva carlosantoniodasilva Update changelogs with version/release dates [ci skip]
Also add note about attr_protected change.
16ed3d5
Commits on Feb 14, 2013
Carlos Antonio da Silva carlosantoniodasilva Fix changelog typos [ci skip]
Thanks to @jmccartie.
967591b
Commits on Feb 16, 2013
joernchen of Phenoelit joernchen Update activemodel/CHANGELOG.md
Fixed a typo ;)
b7ee5ca
Xavier Noria fxn Merge pull request #9309 from joernchen/patch-2
Update activemodel/CHANGELOG.md
7e90a8e
Commits on Feb 27, 2013
Steve Klabnik steveklabnik Revert "Merge pull request #9208 from dylanahsmith/3-2-mysql-quote-nu…
…meric"

This reverts commit 921a296.
2821f95
Josh Owens queso Update gemspec to get mail 2.4 as the main version, 2.3.3 has securit…
…y issues.
d3dc2a7
Commits on Feb 28, 2013
Guillermo Iguaran guilleiguaran Merge pull request #9475 from queso/update-mail
Update gemspec to get mail 2.4 as the main version, 2.3.3 has security i...
3f8eb4e
Commits on Mar 16, 2013
Aaron Patterson tenderlove stop calling to_sym when building arel nodes [CVE-2013-1854] 5ff6012
Charlie Somerville charliesome fix incorrect ^$ usage leading to XSS in sanitize_css [CVE-2013-1855] 36bcc93
benmmurphy benmmurphy JDOM XXE Protection [CVE-2013-1856]
Conflicts:
	activesupport/test/xml_mini/jdom_engine_test.rb
a7d252b
Aaron Patterson tenderlove fix protocol checking in sanitization [CVE-2013-1857]
Conflicts:
	actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb
735bb98
Commits on Mar 18, 2013
Aaron Patterson tenderlove bumping to 3.1.12 0c510c7
Showing with 122 additions and 84 deletions.
  1. +1 −1  RAILS_VERSION
  2. +3 −1 actionmailer/CHANGELOG.md
  3. +1 −1  actionmailer/actionmailer.gemspec
  4. +1 −1  actionmailer/lib/action_mailer/version.rb
  5. +1 −1  actionpack/CHANGELOG.md
  6. +5 −5 actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb
  7. +1 −1  actionpack/lib/action_pack/version.rb
  8. +15 −0 actionpack/test/template/html-scanner/sanitizer_test.rb
  9. +6 −1 activemodel/CHANGELOG.md
  10. +1 −1  activemodel/lib/active_model/version.rb
  11. +8 −1 activerecord/CHANGELOG.md
  12. +2 −8 activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
  13. +1 −1  activerecord/lib/active_record/relation.rb
  14. +1 −5 activerecord/lib/active_record/relation/predicate_builder.rb
  15. +1 −1  activerecord/lib/active_record/version.rb
  16. +5 −5 activerecord/test/cases/method_scoping_test.rb
  17. +7 −7 activerecord/test/cases/quoting_test.rb
  18. +0 −25 activerecord/test/cases/relation/where_test.rb
  19. +3 −3 activerecord/test/cases/relation_scoping_test.rb
  20. +3 −3 activerecord/test/cases/relation_test.rb
  21. +0 −2  activerecord/test/schema/schema.rb
  22. +3 −1 activeresource/CHANGELOG.md
  23. +1 −1  activeresource/lib/active_resource/version.rb
  24. +1 −1  activesupport/CHANGELOG.md
  25. +1 −1  activesupport/lib/active_support/version.rb
  26. +6 −0 activesupport/lib/active_support/xml_mini/jdom.rb
  27. +1 −0  activesupport/test/fixtures/xml/jdom_doctype.dtd
  28. +1 −0  activesupport/test/fixtures/xml/jdom_entities.txt
  29. +1 −0  activesupport/test/fixtures/xml/jdom_include.txt
  30. +36 −3 activesupport/test/xml_mini/jdom_engine_test.rb
  31. +3 −1 railties/CHANGELOG.md
  32. +1 −1  railties/lib/rails/version.rb
  33. +1 −1  version.rb
2  RAILS_VERSION
View
@@ -1 +1 @@
-3.1.11
+3.1.12
4 actionmailer/CHANGELOG.md
View
@@ -1,4 +1,6 @@
-## Rails 3.1.11 (unreleased) ##
+## Rails 3.1.11 (Feb 11, 2011) ##
+
+* No changes.
## Rails 3.1.10 (Jan 8, 2013) ##
2  actionmailer/actionmailer.gemspec
View
@@ -17,5 +17,5 @@ Gem::Specification.new do |s|
s.requirements << 'none'
s.add_dependency('actionpack', version)
- s.add_dependency('mail', '~> 2.3.3')
+ s.add_dependency('mail', '~> 2.4.4')
end
2  actionmailer/lib/action_mailer/version.rb
View
@@ -2,7 +2,7 @@ module ActionMailer
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
2  actionpack/CHANGELOG.md
View
@@ -1,4 +1,4 @@
-## Rails 3.1.11 (unreleased)
+## Rails 3.1.11 (Feb 11, 2011) ##
* Fixed JSON params parsing regression for non-object JSON content.
10 actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb
View
@@ -66,7 +66,7 @@ class WhiteListSanitizer < Sanitizer
# A regular expression of the valid characters used to separate protocols like
# the ':' in 'http://foo.com'
- self.protocol_separator = /:|(&#0*58)|(&#x70)|(%|&#37;)3A/
+ self.protocol_separator = /:|(&#0*58)|(&#x70)|(&#x0*3a)|(%|&#37;)3A/i
# Specifies a Set of HTML attributes that can have URIs.
self.uri_attributes = Set.new(%w(href src cite action longdesc xlink:href lowsrc))
@@ -110,8 +110,8 @@ def sanitize_css(style)
style = style.to_s.gsub(/url\s*\(\s*[^\s)]+?\s*\)\s*/, ' ')
# gauntlet
- if style !~ /^([:,;#%.\sa-zA-Z0-9!]|\w-\w|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*$/ ||
- style !~ /^(\s*[-\w]+\s*:\s*[^:;]*(;|$)\s*)*$/
+ if style !~ /\A([:,;#%.\sa-zA-Z0-9!]|\w-\w|\'[\s\w]+\'|\"[\s\w]+\"|\([\d,\s]+\))*\z/ ||
+ style !~ /\A(\s*[-\w]+\s*:\s*[^:;]*(;|$)\s*)*\z/
return ''
end
@@ -122,7 +122,7 @@ def sanitize_css(style)
elsif shorthand_css_properties.include?(prop.split('-')[0].downcase)
unless val.split().any? do |keyword|
!allowed_css_keywords.include?(keyword) &&
- keyword !~ /^(#[0-9a-f]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|\d{0,2}\.?\d{0,2}(cm|em|ex|in|mm|pc|pt|px|%|,|\))?)$/
+ keyword !~ /\A(#[0-9a-f]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|\d{0,2}\.?\d{0,2}(cm|em|ex|in|mm|pc|pt|px|%|,|\))?)\z/
end
clean << prop + ': ' + val + ';'
end
@@ -171,7 +171,7 @@ def process_attributes_for(node, options)
def contains_bad_protocols?(attr_name, value)
uri_attributes.include?(attr_name) &&
- (value =~ /(^[^\/:]*):|(&#0*58)|(&#x70)|(%|&#37;)3A/ && !allowed_protocols.include?(value.split(protocol_separator).first.downcase))
+ (value =~ /(^[^\/:]*):|(&#0*58)|(&#x70)|(&#x0*3a)|(%|&#37;)3A/i && !allowed_protocols.include?(value.split(protocol_separator).first.downcase.strip))
end
end
end
2  actionpack/lib/action_pack/version.rb
View
@@ -2,7 +2,7 @@ module ActionPack
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
15 actionpack/test/template/html-scanner/sanitizer_test.rb
View
@@ -176,6 +176,7 @@ def test_should_block_script_tag
%(<IMG SRC="jav&#x0A;ascript:alert('XSS');">),
%(<IMG SRC="jav&#x0D;ascript:alert('XSS');">),
%(<IMG SRC=" &#14; javascript:alert('XSS');">),
+ %(<IMG SRC="javascript&#x3a;alert('XSS');">),
%(<IMG SRC=`javascript:alert("RSnake says, 'XSS'")`>)].each_with_index do |img_hack, i|
define_method "test_should_not_fall_for_xss_image_hack_#{i+1}" do
assert_sanitized img_hack, "<img>"
@@ -256,6 +257,11 @@ def test_should_sanitize_div_style_expression
assert_equal '', sanitize_css(raw)
end
+ def test_should_sanitize_across_newlines
+ raw = %(\nwidth:\nexpression(alert('XSS'));\n)
+ assert_equal '', sanitize_css(raw)
+ end
+
def test_should_sanitize_img_vbscript
assert_sanitized %(<img src='vbscript:msgbox("XSS")' />), '<img />'
end
@@ -276,6 +282,15 @@ def test_should_sanitize_neverending_attribute
assert_sanitized "<span class=\"\\", "<span class=\"\\\">"
end
+ def test_x03a
+ assert_sanitized %(<a href="javascript&#x3a;alert('XSS');">), "<a>"
+ assert_sanitized %(<a href="javascript&#x003a;alert('XSS');">), "<a>"
+ assert_sanitized %(<a href="http&#x3a;//legit">), %(<a href="http://legit">)
+ assert_sanitized %(<a href="javascript&#x3A;alert('XSS');">), "<a>"
+ assert_sanitized %(<a href="javascript&#x003A;alert('XSS');">), "<a>"
+ assert_sanitized %(<a href="http&#x3A;//legit">), %(<a href="http://legit">)
+ end
+
protected
def assert_sanitized(input, expected = nil)
@sanitizer ||= HTML::WhiteListSanitizer.new
7 activemodel/CHANGELOG.md
View
@@ -1,4 +1,9 @@
-## Rails 3.1.11 (unreleased) ##
+## Rails 3.1.11 (Feb 11, 2011) ##
+
+* Fix issue with `attr_protected` where malformed input could circumvent protection.
+ CVE-2013-0276
+
+ *joernchen*
## Rails 3.1.10 (Jan 8, 2013) ##
2  activemodel/lib/active_model/version.rb
View
@@ -2,7 +2,7 @@ module ActiveModel
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
9 activerecord/CHANGELOG.md
View
@@ -1,4 +1,11 @@
-## Rails 3.1.11 (unreleased) ##
+## unreleased ##
+
+* Reverted 921a296a3390192a71abeec6d9a035cc6d1865c8, 'Quote numeric values
+ compared to string columns.' This caused several regressions.
+
+ *Steve Klabnik*
+
+## Rails 3.1.11 (Feb 11, 2011) ##
* Quote numeric values being compared to non-numeric columns. Otherwise,
in some database, the string column values will be coerced to a numeric
10 activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
View
@@ -25,19 +25,13 @@ def quote(value, column = nil)
when true, false
if column && column.type == :integer
value ? '1' : '0'
- elsif column && [:text, :string, :binary].include?(column.type)
- value ? "'1'" : "'0'"
else
value ? quoted_true : quoted_false
end
# BigDecimals need to be put in a non-normalized form and quoted.
when nil then "NULL"
- when Numeric, ActiveSupport::Duration
- value = BigDecimal === value ? value.to_s('F') : value.to_s
- if column && ![:integer, :float, :decimal].include?(column.type)
- value = "'#{value}'"
- end
- value
+ when BigDecimal then value.to_s('F')
+ when Numeric then value.to_s
when Date, Time then "'#{quoted_date(value)}'"
when Symbol then "'#{quote_string(value.to_s)}'"
else
2  activerecord/lib/active_record/relation.rb
View
@@ -403,7 +403,7 @@ def where_values_hash
node.left.relation.name == table_name
}
- Hash[equalities.map { |where| [where.left.name, where.right] }]
+ Hash[equalities.map { |where| [where.left.name, where.right] }].with_indifferent_access
end
def scope_for_create
6 activerecord/lib/active_record/relation/predicate_builder.rb
View
@@ -20,7 +20,7 @@ def self.build_from_hash(engine, attributes, default_table, allow_table_name = t
table = Arel::Table.new(table_name, engine)
end
- attribute = table[column.to_sym]
+ attribute = table[column]
case value
when ActiveRecord::Relation
@@ -49,10 +49,6 @@ def self.build_from_hash(engine, attributes, default_table, allow_table_name = t
when Class
# FIXME: I think we need to deprecate this behavior
attribute.eq(value.name)
- when Integer, ActiveSupport::Duration
- # Arel treats integers as literals, but they should be quoted when compared with strings
- column = engine.connection_pool.columns_hash[table.name][attribute.name.to_s]
- attribute.eq(Arel::Nodes::SqlLiteral.new(engine.connection.quote(value, column)))
else
attribute.eq(value)
end
2  activerecord/lib/active_record/version.rb
View
@@ -2,7 +2,7 @@ module ActiveRecord
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
10 activerecord/test/cases/method_scoping_test.rb
View
@@ -212,14 +212,14 @@ def test_scope_for_create_only_uses_equal
table = VerySpecialComment.arel_table
relation = VerySpecialComment.scoped
relation.where_values << table[:id].not_eq(1)
- assert_equal({:type => "VerySpecialComment"}, relation.send(:scope_for_create))
+ assert_equal({'type' => "VerySpecialComment"}, relation.send(:scope_for_create))
end
def test_scoped_create
new_comment = nil
VerySpecialComment.send(:with_scope, :create => { :post_id => 1 }) do
- assert_equal({:post_id => 1, :type => 'VerySpecialComment' }, VerySpecialComment.scoped.send(:scope_for_create))
+ assert_equal({'post_id' => 1, 'type' => 'VerySpecialComment' }, VerySpecialComment.scoped.send(:scope_for_create))
new_comment = VerySpecialComment.create :body => "Wonderful world"
end
@@ -228,7 +228,7 @@ def test_scoped_create
def test_scoped_create_with_join_and_merge
Comment.where(:body => "but Who's Buying?").joins(:post).merge(Post.where(:body => 'Peace Sells...')).with_scope do
- assert_equal({:body => "but Who's Buying?"}, Comment.scoped.scope_for_create)
+ assert_equal({'body' => "but Who's Buying?"}, Comment.scoped.scope_for_create)
end
end
@@ -441,7 +441,7 @@ def test_nested_scoped_create
comment = nil
Comment.send(:with_scope, :create => { :post_id => 1}) do
Comment.send(:with_scope, :create => { :post_id => 2}) do
- assert_equal({:post_id => 2}, Comment.scoped.send(:scope_for_create))
+ assert_equal({'post_id' => 2}, Comment.scoped.send(:scope_for_create))
comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!"
end
end
@@ -453,7 +453,7 @@ def test_nested_exclusive_scope_for_create
Comment.send(:with_scope, :create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do
Comment.send(:with_exclusive_scope, :create => { :post_id => 1 }) do
- assert_equal({:post_id => 1}, Comment.scoped.send(:scope_for_create))
+ assert_equal({'post_id' => 1}, Comment.scoped.send(:scope_for_create))
assert_blank Comment.new.body
comment = Comment.create :body => "Hey guys"
end
14 activerecord/test/cases/quoting_test.rb
View
@@ -122,35 +122,35 @@ def test_quote_false
def test_quote_float
float = 1.2
assert_equal float.to_s, @quoter.quote(float, nil)
- assert_equal float.to_s, @quoter.quote(float, FakeColumn.new(:float))
+ assert_equal float.to_s, @quoter.quote(float, Object.new)
end
def test_quote_fixnum
fixnum = 1
assert_equal fixnum.to_s, @quoter.quote(fixnum, nil)
- assert_equal fixnum.to_s, @quoter.quote(fixnum, FakeColumn.new(:integer))
+ assert_equal fixnum.to_s, @quoter.quote(fixnum, Object.new)
end
def test_quote_bignum
bignum = 1 << 100
assert_equal bignum.to_s, @quoter.quote(bignum, nil)
- assert_equal bignum.to_s, @quoter.quote(bignum, FakeColumn.new(:integer))
+ assert_equal bignum.to_s, @quoter.quote(bignum, Object.new)
end
def test_quote_bigdecimal
bigdec = BigDecimal.new((1 << 100).to_s)
assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, nil)
- assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, FakeColumn.new(:decimal))
+ assert_equal bigdec.to_s('F'), @quoter.quote(bigdec, Object.new)
end
def test_dates_and_times
@quoter.extend(Module.new { def quoted_date(value) 'lol' end })
assert_equal "'lol'", @quoter.quote(Date.today, nil)
- assert_equal "'lol'", @quoter.quote(Date.today, FakeColumn.new(:date))
+ assert_equal "'lol'", @quoter.quote(Date.today, Object.new)
assert_equal "'lol'", @quoter.quote(Time.now, nil)
- assert_equal "'lol'", @quoter.quote(Time.now, FakeColumn.new(:time))
+ assert_equal "'lol'", @quoter.quote(Time.now, Object.new)
assert_equal "'lol'", @quoter.quote(DateTime.now, nil)
- assert_equal "'lol'", @quoter.quote(DateTime.now, FakeColumn.new(:datetime))
+ assert_equal "'lol'", @quoter.quote(DateTime.now, Object.new)
end
def test_crazy_object
25 activerecord/test/cases/relation/where_test.rb
View
@@ -35,30 +35,5 @@ def test_where_with_table_name_and_empty_array
def test_where_with_empty_hash_and_no_foreign_key
assert_equal 0, Edge.where(:sink => {}).count
end
-
- def test_where_with_integer_for_string_column
- count = Post.where(:title => 0).count
- assert_equal 0, count
- end
-
- def test_where_with_float_for_string_column
- count = Post.where(:title => 0.0).count
- assert_equal 0, count
- end
-
- def test_where_with_boolean_for_string_column
- count = Post.where(:title => false).count
- assert_equal 0, count
- end
-
- def test_where_with_decimal_for_string_column
- count = Post.where(:title => BigDecimal.new('0')).count
- assert_equal 0, count
- end
-
- def test_where_with_duration_for_string_column
- count = Post.where(:title => 0.seconds).count
- assert_equal 0, count
- end
end
end
6 activerecord/test/cases/relation_scoping_test.rb
View
@@ -380,19 +380,19 @@ def test_default_scoping_with_threads
def test_default_scope_with_inheritance
wheres = InheritedPoorDeveloperCalledJamis.scoped.where_values_hash
assert_equal "Jamis", wheres[:name]
- assert_equal Arel.sql("50000"), wheres[:salary]
+ assert_equal 50000, wheres[:salary]
end
def test_default_scope_with_module_includes
wheres = ModuleIncludedPoorDeveloperCalledJamis.scoped.where_values_hash
assert_equal "Jamis", wheres[:name]
- assert_equal Arel.sql("50000"), wheres[:salary]
+ assert_equal 50000, wheres[:salary]
end
def test_default_scope_with_multiple_calls
wheres = MultiplePoorDeveloperCalledJamis.scoped.where_values_hash
assert_equal "Jamis", wheres[:name]
- assert_equal Arel.sql("50000"), wheres[:salary]
+ assert_equal 50000, wheres[:salary]
end
def test_method_scope
6 activerecord/test/cases/relation_test.rb
View
@@ -71,7 +71,7 @@ def test_empty_where_values_hash
def test_has_values
relation = Relation.new Post, Post.arel_table
relation.where_values << relation.table[:id].eq(10)
- assert_equal({:id => 10}, relation.where_values_hash)
+ assert_equal({'id' => 10}, relation.where_values_hash)
end
def test_values_wrong_table
@@ -101,7 +101,7 @@ def test_scope_for_create
def test_create_with_value
relation = Relation.new Post, Post.arel_table
- hash = { :hello => 'world' }
+ hash = { 'hello' => 'world' }
relation.create_with_value = hash
assert_equal hash, relation.scope_for_create
end
@@ -110,7 +110,7 @@ def test_create_with_value_with_wheres
relation = Relation.new Post, Post.arel_table
relation.where_values << relation.table[:id].eq(10)
relation.create_with_value = {:hello => 'world'}
- assert_equal({:hello => 'world', :id => 10}, relation.scope_for_create)
+ assert_equal({'hello' => 'world', 'id' => 10}, relation.scope_for_create)
end
# FIXME: is this really wanted or expected behavior?
2  activerecord/test/schema/schema.rb
View
@@ -486,8 +486,6 @@ def create_table(*args, &block)
create_table :price_estimates, :force => true do |t|
t.string :estimate_of_type
t.integer :estimate_of_id
- t.string :thing_type
- t.integer :thing_id
t.integer :price
end
4 activeresource/CHANGELOG.md
View
@@ -1,4 +1,6 @@
-## Rails 3.1.11 (unreleased) ##
+## Rails 3.1.11 (Feb 11, 2011) ##
+
+* No changes.
## Rails 3.1.10 (Jan 8, 2013) ##
2  activeresource/lib/active_resource/version.rb
View
@@ -2,7 +2,7 @@ module ActiveResource
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
2  activesupport/CHANGELOG.md
View
@@ -1,4 +1,4 @@
-## Rails 3.1.11 (unreleased) ##
+## Rails 3.1.11 (Feb 11, 2011) ##
* Allow `multi_json` version `>= 1.3`, relaxing back to semantic versioning 2.0.0 (revert of #5861)
Backport of #5896
2  activesupport/lib/active_support/version.rb
View
@@ -2,7 +2,7 @@ module ActiveSupport
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
6 activesupport/lib/active_support/xml_mini/jdom.rb
View
@@ -38,6 +38,12 @@ def parse(data)
{}
else
@dbf = DocumentBuilderFactory.new_instance
+ # secure processing of java xml
+ # http://www.ibm.com/developerworks/xml/library/x-tipcfsx/index.html
+ @dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false)
+ @dbf.setFeature("http://xml.org/sax/features/external-general-entities", false)
+ @dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false)
+ @dbf.setFeature(javax.xml.XMLConstants::FEATURE_SECURE_PROCESSING, true)
xml_string_reader = StringReader.new(data)
xml_input_source = InputSource.new(xml_string_reader)
doc = @dbf.new_document_builder.parse(xml_input_source)
1  activesupport/test/fixtures/xml/jdom_doctype.dtd
View
@@ -0,0 +1 @@
+<!ENTITY a "external entity">
1  activesupport/test/fixtures/xml/jdom_entities.txt
View
@@ -0,0 +1 @@
+<!ENTITY a "hello">
1  activesupport/test/fixtures/xml/jdom_include.txt
View
@@ -0,0 +1 @@
+include me
39 activesupport/test/xml_mini/jdom_engine_test.rb
View
@@ -3,9 +3,11 @@
require 'active_support/xml_mini'
require 'active_support/core_ext/hash/conversions'
- class JDOMEngineTest < Test::Unit::TestCase
+ class JDOMEngineTest < ActiveSupport::TestCase
include ActiveSupport
+ FILES_DIR = File.dirname(__FILE__) + '/../fixtures/xml'
+
def setup
@default_backend = XmlMini.backend
XmlMini.backend = 'JDOM'
@@ -30,10 +32,41 @@ def test_file_from_xml
assert_equal 'image/png', file.content_type
end
+ def test_not_allowed_to_expand_entities_to_files
+ attack_xml = <<-EOT
+ <!DOCTYPE member [
+ <!ENTITY a SYSTEM "file://#{FILES_DIR}/jdom_include.txt">
+ ]>
+ <member>x&a;</member>
+ EOT
+ assert_equal 'x', Hash.from_xml(attack_xml)["member"]
+ end
+
+ def test_not_allowed_to_expand_parameter_entities_to_files
+ attack_xml = <<-EOT
+ <!DOCTYPE member [
+ <!ENTITY % b SYSTEM "file://#{FILES_DIR}/jdom_entities.txt">
+ %b;
+ ]>
+ <member>x&a;</member>
+ EOT
+ assert_raise Java::OrgXmlSax::SAXParseException do
+ assert_equal 'x', Hash.from_xml(attack_xml)["member"]
+ end
+ end
+
+
+ def test_not_allowed_to_load_external_doctypes
+ attack_xml = <<-EOT
+ <!DOCTYPE member SYSTEM "file://#{FILES_DIR}/jdom_doctype.dtd">
+ <member>x&a;</member>
+ EOT
+ assert_equal 'x', Hash.from_xml(attack_xml)["member"]
+ end
+
def test_exception_thrown_on_expansion_attack
- assert_raise NativeException do
+ assert_raise Java::OrgXmlSax::SAXParseException do
attack_xml = <<-EOT
- <?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE member [
<!ENTITY a "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;">
<!ENTITY b "&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;">
4 railties/CHANGELOG.md
View
@@ -1,4 +1,6 @@
-## Rails 3.1.11 (unreleased) ##
+## Rails 3.1.11 (Feb 11, 2011) ##
+
+* No changes.
## Rails 3.1.10 (Jan 8, 2013) ##
2  railties/lib/rails/version.rb
View
@@ -2,7 +2,7 @@ module Rails
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')
2  version.rb
View
@@ -2,7 +2,7 @@ module Rails
module VERSION #:nodoc:
MAJOR = 3
MINOR = 1
- TINY = 11
+ TINY = 12
PRE = nil
STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.')

Showing you all comments on commits in this comparison.

Peter Marreck

I'm thinking that a hypothetical HashEnforcingSymbolKeys and a with_enforced_symbol_keys would have prevented the unintended side effect of the scopes overwriting other scopes bug in #9813 :)

It just seems like more, instead of less, "key discipline" is needed. This is not the first time, btw, that I've seen weird bugs after manipulation of hash objects between string and symbol keys (or agnosticizing via .with_indifferent_access) or, for example, wrapping objects mixing both hash types in a wrapper class which one then tries to set values through (we specifically saw a nasty bug in our own code which only occurred when wrapping a middleware env hash in an ActionDispatch::Request and then setting values through it, for example... Entire sessions got lost, randomly!). I have become very suspicious of HashWithIndifferentAccess due to that ambiguity. The cost of access convenience (coupled with mutability) seems to be a major recipe for unintended side effects.

I put some corner-case examples demonstrating my concern here: https://gist.github.com/pmarreck/5205946
Notice how Hash[] is not overwritten correctly by HashWithIndifferentAccess[]=, allowing symbol keys to sneak in, probably directly leading to this bug. But I also have general concerns around merges.

Bert Goethals

@pmarreck solution sounds good. Demanding that the keys in a hash style query would actually make things a lot simpler.

  • the framework will not call .to_sym any longer, so no DOS attack
  • if the key is not a Symbol, it's not a column, it's a value or it errors. This avoids SQL injection
Peter Marreck

Well, browsers can't send symbols in forms :) , so a conversion would still have to happen somewhere, but it should happen once and once only and in one location and not be scattered all willy-nilly around the code haphazardly (first it's a symbol, then it's a string, then it doesn't care because it's with_indifferent_access, then it's merged with a hash with symbol keys, or string keys, etc.)

I am thinking that once you do that conversion, in one place, that's it. Everything's either symbols or strings from there on out and raises otherwise if you try to merge with a hash that contains keys of a differing key class or you try to set or get via the wrong key class (to be strict, and avoid this entire class of bugs).

Considerations: weird symbol possibilities like :"a[weird_symbol]" and the fact that symbols are never garbage-collected.

Bert Goethals

Well, that's the thing. If we assume the browser doesn't send symbols ( they can, we've sen that ) and we demand that Hash type where's should use symbols for column names; then the developer has to do the casting.
This makes it safe from the frameworks point of view.

Something went wrong with that request. Please try again.