This project is archived and is in readonly mode.

#6084 new
C. Bedard

ActiveRecord Association Proxy/Collection create method incorrectly merges attributes

Reported by C. Bedard | November 29th, 2010 @ 08:58 PM

Given the following associated models:

class Client < ActiveRecord::Base
  has_many :orders

class Order < ActiveRecord::Base
  belongs_to :client

And let's say I have an orders controller in which a Client creates their orders:

class OrdersController < ApplicationController
  def create

This mass-assignment, in Rails 2.X.X, would always ensure that the Order Object belonged to the association owner, by forcing the foreign key (client_id) to that of @client.

With Rails 3.0.3, the behavior has changed, making it possible to inject a foreign key simply by setting it. So, in this example, if params[:order] happens to have a :client_id key, that value will override the associations's owner id. In other words, the AR scope that is specified by @client.orders doesn't enforce itself when creating associated objects.

This may also represent a "security" vulnerability, as anyone can easily infer the foreign key column name (Rails makes it pretty easy) and inject a value submitted in a form, that can create objects belonging to other objects that should not be acessible in this way.

Also, calling[:order]) does enforce the correct scope attributes (as would be expected).

I am not familiar with the internals of the new AR querying interface, but having investigated a bit, it all seems to originate in ActiveRecord::Base#with_scope, where improper merging of attributes occurs with construct_scope attributes.

Comments and changes to this ticket

  • C. Bedard

    C. Bedard November 30th, 2010 @ 07:59 PM

    • Tag changed from activerecord rails3, associations, build, create, scope, with_scope to activerecord rails3, associations, build, create, patch, scope, with_scope

    More on this issue:

    I initially thought the problem was in ActiveRecord::Base#with_scope but now after more digging, I found out the problem originates in ActiveRecord::Base#initialize, where 2 lines should be reversed in order for scope attributes to be assigned after the attributes Hash that is received as a parameter. Currently the method goes like this:

    def initialize(attributes = nil)
      self.attributes = attributes unless attributes.nil?
      result = yield self if block_given?

    The reason is simple: as it is right now, the scope attributes are first assigned, but then attributes are overwritten with the attributes Hash passed as a parameter. Reversing those to lines solves the problem.

    Attached is the patch acting on ActiveRecord::Base#initialize, which simply assigns scope attributes after the attributes parameter, therby enforcing the scope attributes over the parameter attributes.

    This is, by the way, the way it was done in Rails 2. Maybe there is a reason for this in Rails 3, but it is not readily apparent.

  • Mike Ragalie

    Mike Ragalie December 4th, 2010 @ 07:28 PM

    I confirm that this bug exists in edge. And it is potentially a problem, since I can see someone doing something like this:

    post = Post.where(:title => "My Favorite Things")
    post_dup = Post.where(:title => "My Favorite Thingz")
    comment = post_dup.comments.first

    However, the above patch breaks other AR functionality (some of the tests fail) so I created tests for the problem and I've attached a new fix that passes all tests. In short, the build methods on has_one/has_many call #set_belongs_to_association_for but the create methods do not. I refactored the has_one create methods to operate similarly to the has_many create methods and added in a call to #set_belongs_to_association_for for both association types.

  • Jesse Storimer

    Jesse Storimer January 29th, 2011 @ 04:01 AM

    +1 for Mike's patch. I definitely agree that this is unexpected behaviour and might raise a security eyebrow.

  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>

People watching this ticket