Skip to content

Commit

Permalink
Check attributes passed to create_with and where
Browse files Browse the repository at this point in the history
If the request parameters are passed to create_with and where they can
be used to do mass assignment when used in combination with
Relation#create.

Fixes CVE-2014-3514
  • Loading branch information
rafaelfranca committed Aug 18, 2014
1 parent 7c4bfe1 commit 9456990
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ def sanitize_for_mass_assignment(attributes)
attributes
end
end
alias :sanitize_forbidden_attributes :sanitize_for_mass_assignment
end
end
16 changes: 14 additions & 2 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
require 'active_support/core_ext/array/wrap'
require 'active_model/forbidden_attributes_protection'

module ActiveRecord
module QueryMethods
extend ActiveSupport::Concern

include ActiveModel::ForbiddenAttributesProtection

# WhereChain objects act as placeholder for queries in which #where does not have any parameter.
# In this case, #where must be chained with #not to return a new relation.
class WhereChain
Expand Down Expand Up @@ -561,7 +564,10 @@ def where!(opts = :chain, *rest) # :nodoc:
if opts == :chain
WhereChain.new(self)
else
references!(PredicateBuilder.references(opts)) if Hash === opts
if Hash === opts
opts = sanitize_forbidden_attributes(opts)
references!(PredicateBuilder.references(opts))
end

self.where_values += build_where(opts, rest)
self
Expand Down Expand Up @@ -711,7 +717,13 @@ def create_with(value)
end

def create_with!(value) # :nodoc:
self.create_with_value = value ? create_with_value.merge(value) : {}
if value
value = sanitize_forbidden_attributes(value)
self.create_with_value = create_with_value.merge(value)
else
self.create_with_value = {}
end

self
end

Expand Down
30 changes: 30 additions & 0 deletions activerecord/test/cases/forbidden_attributes_protection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,34 @@ def test_blank_attributes_should_not_raise
person = Person.new
assert_nil person.assign_attributes(ProtectedParams.new({}))
end

def test_create_with_checks_permitted
params = ProtectedParams.new(first_name: 'Guille', gender: 'm')

assert_raises(ActiveModel::ForbiddenAttributesError) do
Person.create_with(params).create!
end
end

def test_create_with_works_with_params_values
params = ProtectedParams.new(first_name: 'Guille')

person = Person.create_with(first_name: params[:first_name]).create!
assert_equal 'Guille', person.first_name
end

def test_where_checks_permitted
params = ProtectedParams.new(first_name: 'Guille', gender: 'm')

assert_raises(ActiveModel::ForbiddenAttributesError) do
Person.where(params).create!
end
end

def test_where_works_with_params_values
params = ProtectedParams.new(first_name: 'Guille')

person = Person.where(first_name: params[:first_name]).create!
assert_equal 'Guille', person.first_name
end
end

0 comments on commit 9456990

Please sign in to comment.