Rails Mass Assignment Issue: A PHP Perspective

Now more than ever is the security of your rails application under close scrutiny. Github user homakov (Egor Homakov exposed a vulnerability of Rails (that no doubt also lives in an app near you) with this playful commit in the Rails project itself.. I fell in love with this commit purely for the meme fest that ensued. However, the reaction from the Rails community in general has left me a little surprised. Since the attack, there have been countless solutions posted on blogs ranging from trivial to DSL extravagance.

So What Happened?

In case you missed all the hoo-ha, the mass assignment vulnerability has lived in Rails or more specifically ActiveRecord since year dot. Every time you write code in a controller such as:

[gist id=”2072178″]

You have opened yourself up to the now almost infamous hack. Mass assignment is convenient, in fact it’s so convenient I shudder at the prospect of updating model attributes any other way via forms. I will be first to stand up and admit my shame (or lack of) in the fact I have written/generated code like this, and it’s still in production today.

My background is in PHP and most small scale applications I wrote were sans framework. I would reuse a database class that handled all the nuts and bolts of of reading and writing to the databse, then I simply inherited my model code from that. Sound familiar? Now all that reading and writing was handled via PHP Data Objects (PDO). Because of my experience with PDO, this article should really be called “Mass assignment vulnerability in Rails and how PHP PDO made sure I was never caught out like Github”, but that is verging on the controversial let alone ridiculous.

PDO 101

For those unaware of PDO, it is a layer of abstraction between my code and the database. Much like ActiveRecord regardless of what database I end up using, by implementing my data access via PDO I can shift between SQLite, MySQL, PostgreSQL etc. One of the fundamental components of using PDO is prepared statements. A prepared statement looks something like:

[gist id=’2072181′]

What we have here is a snippet of a function that accepts two parameters ($some_variable and $another_variable), these are subsequently escaped and injected into our SQL statement.

In Rails, or more specifically ActiveRecord (even more specifically ActiveModel) this is probably the equivalent to the silver bullet that is attr_accessible. Much like attr_accessible in the above prepared statement, we have essentially white listed what parameters will be used with our SQL statement.

The same protection in a Rails application would look like:

class SomeModel < ActiveRecord::Base attr_accessible :first_name, :surname end

In both PHP and Rails versions, if the model SomeModel had an is_admin attribute, it would be protected from mass assignment. The only real difference, and it’s significant, is the far fewer lines of code we actually have to type. Yes, Rails is a framework/eco-system, so that will be the case. I just wished for a moment all that PDO code when I wrote it many moons ago was as terse.

The point is, in relation to the mass assignment vulnerability, it would never have happened using rudimentary, simple, data access as we see in the PHP prepared statement. Is there an argument that the ease of ActiveRecord has bred complacency? My opinion is that for novice rubyists, certainly (although they will be blissfully unaware of the fact) and for the more experienced among us, probably. However, I’ll let you be the real judge of that proposition.

All Done, Not Quite

Up to this point, all that has been discussed is the mechanics of placing data into the database. Is that where the smoking gun for this vulnerability lies? We know how to prevent it now, we always have known, it’s even commented in every recent Rails application we create. In our config/application.rb file we have the commented out line :

# config.active_record.whitelist_attributes = true

By uncommenting that one line, we are safe, phew. But the application is probably going to break. It is at this point we see the real culprits, and they don’t live in models. There has been a frenzy in creating new, almost elaborate techniques to protect from mass assignment using attr_accessible or kissing cousin attr_protected, while the real offenders are still at large. The following code, in controllers, is convenient but wrong .

[gist id=’2072203′]

There are two offending items here. First of all, by using update_attributes or save in a controller breaks the first law of SOLID code, single responsibility. But hey, it’s Rails we do this kind of stuff every day without even a nod to the SOLID principles. We would only extract this kind of thing to the model anyway.

Now we are at my main gripe with this code snippet. The parameters, passing params[:whatever] directly to a method on a model is a real shoddy piece of work. It’s lazy and reckless. We are all expecting ActiveRecord to do the hard work for us and just work while we happily pass it external data.

Murder She Wrote

First of all, I’m more a Columbo (early stuff) kind of guy, but I’m sure pretty much everyone reading this will be thinking we have already covered a sufficient remedy for accepting the params hash into a model in the form of attr_accessible. Yet I have never used attr_accessible in a single application. I know a couple of ‘quick, dirty’ apps are still susceptible to mass assignment attacks but there are a few which I have secured, without using attr_accessible or attr_protected.

Remember I mentioned the article title should be along the lines of how my experience with PDO would protect me from mass assignment attacks in Rails? Well, back in the day I yearned for mass assignment, binding each parameter for a PDO prepared statement is tedious at best. So I wrote a helper method.

[gist id=”2072695″]

The execute method for prepared statements accepts an optional associative array (hash if you do ruby), thus I could get the convenience of mass assignment passing the result of build_params to the data access class.

[gist id=”2072728″]

Of course, this was not without it’s flaws. I am back in the position of being open to mass assignment attacks, and every element (submit buttons etc. anything in the form that had a named attribute) was being passed to the prepared statement. The fix was easy enough.

[gist id=’2072748′]

I have protection again from mass assignment and, more importantly, the controller is responsible for that protection. When it came to learning and using rails I took this simple idiom with me.

I was going to post my example code in Ruby (basically a straight port of the PHP) then I noticed DHH posted a gist with the same functionality but nicer, using Array#slice.

[gist id=”2072787″]

It really becomes a matter of taste, belief of where the logic for mass assignment protection lies. Naturally, I would never frown upon someone using attr_accessible. In fact, I like the simplicity of white listing all attributes in the model itself. But always in the back of my mind I hear the voice of Jim Weirich reminding me about SOLID principles and the single responsibility that is often lost in Rails for convenience. I am of the feeling a model should not be responsible for the data it is passed. If controller is the “glue” between our models and the views, surely that is where the responsibility lies?

Of course, I could not finish this article without pointing out some great resources for security, specifically with Rails. Feel free to add any more in the comments.