Skip to content

Rails 3 Mass Assignment Error Message

How I Avoid The Rails Mass-Assignment Security Mistake

Note to readers from the future: this is a non-issue now that by default, rails complains abouts calling model#update_attributes without first setting up your attr_accessors

Wat?

By default, rails allows you to do this in your controller:

Where are your http request parameters updates a record in your database.

If it's not obvious why this is more than a little wreckless, let's give you a more concrete example.

# models/order.rb class Order # has a user_id field, # to determine the customer that this order belongs to belongs_to :user end # controllers/orders_controller.rb class OrdersController def update order = Order.find(params[:id]) if current_user == order.user order.update_attributes(params) else raise 'Stop trying to modify other users orders!' end end end

At first glance that code looks secure because you're authorizing your user (In practice you would use a gem like cancan that makes authorization a lot cleaner). It does make sure you can only modify your own orders.

But it doesn't whitelist exactly which fields you can modify. Imagine you sent these fields to that endpoint:

You've assigned your order to another user!

Look familiar? It should. A certain rather popular company with exactly one metric buttload of developers as users fell prey to this exact bug. I'm a big fanboy of theirs so I won't repeat exactly who it was, but they're huge.

Messeurs Picard and Riker sum up the feeling when you discover a bug like this in your code:

This is not a bug in rails. It's just a bit of functionality that makes it quite easy to stab yourself in the face.

Rails also gives you an easy way to white list attributes that can be mass assigned in the class method:

# models/order.rb class Order belongs_to :user attr_accessible :created_at, :address_id # and so on end

The result of this is that if you try to mass assign anything that isn't declared with , an exception is thrown, your app assplodes and that is the right thing.

The whitelist only comes into effect when you call on at least one attribute.

This means that in practice we neglect to whitelist anything and tend to leave this sort of security vulnerability lying around in our Rails codebases.

Guilty until proven innocent

On any new project then, I add this initializer:

# app/initializers/disable_mass_assignment.rb ActiveRecord::Base.attr_accessible(nil)

This automatically switches on whitelisting for all your model objects, making sure I whitelist all of my models before I mass assign them.

(Note, according to the relevant rails guide you can achieve the same effect with the following config option in ):

Use Only What You Need

Even with that in place and, it feels a little, well, wrong to just stick query parameters unchecked into . I'm with DHH on filtering your query parameters like so:

It's definitely a controller concern, though I wouldn't hesitate to move the fine details of the filtering logic somewhere else if it got to be more then four or five lines of code.

I have used RESTful techniques to generate a model (in fact, I am using Devise gem, which does that for me), and I have added new fields called first_name and last_name to the model. Migration went fine. I added attr_accessor :first_name, :last_name to the model and expected it would just work. But when I try to mass-assign new instances with Doctor.create({:first_name=>"MyName"}) etc., I am getting errors saying I can't mass-assign protected attributes.

I thought the whole point of using attr_accessor was to get around the protectedness of the fields of a model. Can you help me make sense of this message?

Edit: oh, and by the way the records do not get created either. I thought they should be since this is just a warning, but they are not on the database.

Edit2: here is my model

and the schema, which doesn't have the first_name and last_name because they are created in the users table, which is the ancestor of doctors. I used single table inheritance.

and this is the migration to change the users table

EDIT: here is the seed file. I am not including the truncate_db_table method, but it works.

ruby-on-railsactiverecorddevise