Skip to content

Commit

Permalink
Additional fix for CVE-2012-2661
Browse files Browse the repository at this point in the history
While the patched PredicateBuilder in 3.1.5 prevents a user
from specifying a table name using the `table.column` format,
it doesn't protect against the nesting of hashes changing the
table context in the next call to build_from_hash. This fix
covers this case as well.
  • Loading branch information
ernie authored and tenderlove committed Jun 11, 2012
1 parent 0ccdeeb commit cc2903d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/predicate_builder.rb
@@ -1,16 +1,16 @@
module ActiveRecord
class PredicateBuilder # :nodoc:
def self.build_from_hash(engine, attributes, default_table, check_column = true)
def self.build_from_hash(engine, attributes, default_table, allow_table_name = true)
predicates = attributes.map do |column, value|
table = default_table

if value.is_a?(Hash)
if allow_table_name && value.is_a?(Hash)
table = Arel::Table.new(column, engine)
build_from_hash(engine, value, table, false)
else
column = column.to_s

if check_column && column.include?('.')
if allow_table_name && column.include?('.')
table_name, column = column.split('.', 2)
table = Arel::Table.new(table_name, engine)
end
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/relation/where_test.rb
Expand Up @@ -11,6 +11,12 @@ def test_where_error
end
end

def test_where_error_with_hash
assert_raises(ActiveRecord::StatementInvalid) do
Post.where(:id => { :posts => {:author_id => 10} }).first
end
end

def test_where_with_table_name
post = Post.first
assert_equal post, Post.where(:posts => { 'id' => post.id }).first
Expand Down

2 comments on commit cc2903d

@kennyj
Copy link
Contributor

@kennyj kennyj commented on cc2903d Jun 13, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ernie @tenderlove
Please see #6718.

I guess specification's conflict has occurred.

@ernie
Copy link
Contributor Author

@ernie ernie commented on cc2903d Jun 13, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kennyj posting response to that issue now. Short version -- code that treated nested hashes this was is wrong. Long version will be posted shortly.

Please sign in to comment.