If you didn't catch all of the action on Github last week - where have you been?
In this post, I'd like to share my opinion on Rail's inherent vulnerability to mass-assignment attacks and suggest a solution which should be applied to Rails to help make Rails more secure for all of us.
Who's job is it anyway?
First up, I don't think this is a problem with Rails; I think this is a problem with Rails developers!
Rails is sometimes too convenient for us. Because Rails handles so much behind the scenes we can often miss out on some of the essential responsibilities we have when writing software.
While others have suggested solutions for improving the mass-assignment situation in Rails (including Yehuda Katz and Technoweenie), I don't agree that we need to change how we assign attributes from query parameters. The current Rails approach was a perfectly viable solution. The problem is that not everybody was using it!
Any n00b who's watched a few of Ryan Bates's (fantastic) Railscasts knows that sensitive attributes should be protected from mass-assignment. Yet from experience working with my own code and other seasoned developers, it's a point we often miss (along with applying constraints to database columns - another bugbear for a separate post).
In short, the solution to this problem is not to dramatically change Rails. The solution is to notify developers while they're using Rails.
My Proposal for Improving Mass Assignment in Rails
attr_protected methods offer perfectly adequate protection against mass assignment when creating or updating records from the
params hash. The real problem lies in Rails developers overlooking their importance in the architecture of the system.
Every ActiveRecord::Base subclass is mass-assignment vulnerable by default.
What if we were to add this as an attribute to each ActiveRecord::Base subclass?
class User < ActiveRecord::Base self.mass_assignment_safe? # => false end
save is called on an instance of
User, a warning is printed to the log to alert the developer that they haven't protected the model from mass-assignment attacks. Something along the lines of
[Warning] User is not protected against mass-assignment attacks. Please use either attr_protected or attr_accessible.
attr_accessible within the class would set
mass_assignment_safe? to true
class User < ActiveRecord::Base self.mass_assignment_safe? # => false attr_accessible :first_name, :last_name, :email, :password # or ... attr_protected :created_at, :updated_at, :admin self.mass_assignment_safe? # => true end
This would ensure that developers cannot inadvertently overlook applying at least some mass assignment protection to their model.
Of course there will be some models where no attributes should be protected. In these cases, having this warning message cluttering up the console a hundred times each time the test suite is run or showing up in the log all the time would be a pain in the ass.
To avoid this, the developer can explicitly state that the model should allow all attributes to be mass-assigned by passing
:all => true to attr_accessible or
:none => true to attr_protected.
class User < ActiveRecord::Base attr_protected :none => true # or ... attr_accessible :all => true # BTW ... attr_accessible :all => true, :username # => raises an exception! attr_accessible :none => true # => raises an exception too! end
It's an obvious and simple solution but it requires no extra work from developers to implement, it won't break any existing code, and unlike the other suggestions I've seen it's DRY.
What's more, it keeps the implementation logic for attribute protection in the model where it belongs.