Merb's approach was to have mass assignment protection in the controller, and I personally think it's self-evident that it belongs there. Moving it into the controller will also make it easier to solve the tension between reducing the friction of getting up and running quickly and having good security defaults.
In general, Rails' convention over configuration make a stock Rails app more secure by default (CSRF protections, XSS protection, timing attacks, session fixation, etc.). This is a case where there's a real tension, but I think that we can solve it by applying some brainpower to the question.
It is. The more interesting question is why it takes rails 7 years (and counting) to come to this conclusion.
<rant>
I'll take it one further and sing my song about the Rails ActiveRecord implementation here, which is tangentially related.
The promise of AR is to reflect on the database at startup and then "magically work".
The problem in rails is that nothing magically works. What you get out of the box is an insecure world-writable model, as illustrated by this bug. Then you begin scattering your truth over incremental (hand-crafted!) migrations, the model, and the controller, until nobody other than a cascade of unit-tests can even make sense of the interdependencies anymore.
In a world where there is not even a central place to look up which fields exist on a model and what their constraints are - short of runtime introspection, where database constraints live happily alongside and independently of model constraints, where opaque bits of ruby-logic buried in various gems add their own magic to the mix, in such a world it's really no surprise they chose to default to "all fields writable".
Because if they forced the user to do the sensible thing and explicitly list the allowable fields then where's the advantage over a declarative approach (like e.g. Django) anymore?
Imho it's long overdue to take a step back and revisit whether AR is still worth having, or ever was. Imho it causes way more problems than it solves, in contrast to the declarative approach.
No downvote here. I'm by far no Rails guru - I've done one moderate-sized project in it (as an apprentice to someone far more experienced) and a couple of smallish projects, Rails feels brittle. I thought I was pretty alone in that thinking, but based on your rant above, I'm not 100% on my own. I've gotten used to Grails/GORM, which has its own set of issues, for sure, but has always felt more natural.
While not mitigating most of the issues you listed, I thought I would mention the awesome https://github.com/ctran/annotate_models plugin in case you did not know about it. I personally could not work on Rails without it.
AR was born with a set of very opinionated decisions. I believe those that prefer a more declarative approach (and built-in identity map) can use DataMapper.
What do you think about the solution proposed in by homakov (the "hacker")? Just mark all the *_id attributes "protected" by default. Seems elegant and really easy to implement and fixes 90% of the problem.
I agree completely. The attr_accessible issue has been around for so long (I blogged about it in 2007: http://news.ycombinator.com/item?id=1031168), I'd assume it is by far one of the easiest things to exploit in a production Rails application. Personally, if I wanted to hack a Rails application, this is the first thing I'd try.
Your proposed controller technique is great. This technique coupled with some intelligent defaults seems like a good idea, with respect to both sides of the story (both a secure-by-default and frictionless convention). I'm excited to see this in the limelight, and your proposal is a fantastic start to this conversation. Thank you sir!
Maybe the right proposal is just to disable mass assignment by default? Meaning there is already white listing functionality - always specify attributes you want to save.
Currently, I like to use a hack that automatically makes all models use attr_accessible with no allowed attributes. Until you override it with attr_accessible in your model, nothing is allowed, so you have to think hard about what should and shouldn't be accessible.
However, it causes real annoyance when not dealing with web input. Applying it to an existing project and fixing everything it breaks is super-annoying.
Right, refactoring an existing project to enforce attr_accessible nil by default can be a hassle.
From my experience, the main (though easily side-stepped) annoyance is when creating or updating records that have belongs_to associations (for example, user_id and repository_id for a commit ;)) programmatically.
For security purposes, you would not set those 2 attributes to be attr_accessible. To create a new record, you then would have to build the record and then set the user_id and repository_id on the record.
Or, you can set user and repository to be accessible (attr_accessible :user, :repository). This is fine because the associated methods expect ActiveRecord objects.
Not every idea from Merb made it into Rails. Especially when an idea would cause significant backwards-compatibility breakage (return string from action vs. implicit rendering), we stuck with the Rails approach.
I always felt that "it's up to the developer to do the right thing" violates the normal Rails convention over configuration principles, but I also weigh breaking a large % of existing Rails apps in a way that is not easy to quickly fix heavily.
That said, this problem is almost identical to XSS protection. We were able to find a solution that mostly "just works" for new developers, with some caveats, but it broke nearly all existing apps in a way that required significant effort to fix.
Like mass assignment, previous vulnerabilities were caused by Rails defaults that caused most users to make mistakes (nearly everyone had at least a few cases where `h` was required but wasn't done).
Like XSS protection, we have a solution here that will mostly just work for the happy path. The end result is a Rails default that will be only marginal harder to use than what we have now, but secure by default.
>Like mass assignment, previous vulnerabilities were caused by Rails defaults that caused most users to make mistakes (nearly everyone had at least a few cases where `h` was required but wasn't done).
But let me just add, fixing those when I moved rails 3 felt good.
> I always felt that "it's up to the developer to do the right thing" violates the normal Rails convention over configuration principles, but I also weigh breaking a large % of existing Rails apps in a way that is not easy to quickly fix heavily.
It can be argued that those apps were already broken. Nobody should complain against a security fix.
Yes, the way Merb handled controllers was one of places it really shined over Rails. Hopefully over time more and more of these ideas will make their way back into to Rails.
I don't have a well-formed opinion on this, but how would you propose handling nested attributes if mass assignment protection was managed in the controller?
Merb's approach was to have mass assignment protection in the controller, and I personally think it's self-evident that it belongs there. Moving it into the controller will also make it easier to solve the tension between reducing the friction of getting up and running quickly and having good security defaults.
In general, Rails' convention over configuration make a stock Rails app more secure by default (CSRF protections, XSS protection, timing attacks, session fixation, etc.). This is a case where there's a real tension, but I think that we can solve it by applying some brainpower to the question.