Is anyone else laughing at how ridiculous this vulnerability is?<p>I just spent a few hours last week hacking through the Stripe CTF game. Environment variables, string formatting injections, and a timing/side channel attack to top it off.<p>This is just POSTing a value to an endpoint. And it gets written?! To the database?! That's awesome and scary at the same time.
I wonder if they pay their security firms for code audits, vs. just system configuration level stuff. It's reasonable to think they're about as expert on their codebase and its security as anyone else would be, so just having different people within the same company look at it would probably be effective, and could be done more frequently than an external audit. (I generally advise AGAINST external code audit for a lot of early stage companies, since it can be a lot of wasted money when the codebase is changing; it's better to have some guiding principles internally and then try to reduce the scope of security critical code as much as possible, and eventually audit that. Stick to securing the infrastructure, which is pretty standard across companies and thus cheap, and can be handled through outsourcing to gmail/heroku/etc., at least in the beginning.)<p>Looks like they have two companies for audit/pentest (nGenuity and Matasano Security), plus a consultant. Somehow I doubt tptacek is going to comment on any of this.
I'm not a fan of some of the comments about the man posting, you'd think the developer community would be above "you look like frankenstein" and "your english sucks."
To be honest, mass assignment sounds like Rails' own "register_globals". The default should be conservative and disallow setting any fields, instead of allowing anything to be changed.
I just threw together a quick gem that will ensure active_record model attributes are protected from mass-assignment unless explicitly declared as mass-assignable.<p>Granted, this as default will break an app that does not have the correct attributes declared as mass-assignable, but the alternative is a vulnerable app.<p><a href="https://github.com/stevegraham/default_whitelist" rel="nofollow">https://github.com/stevegraham/default_whitelist</a>
I'm not too familiar with RoR, but does the mass assignment vulnerability basically boil down to not doing any input validation for the convenience of updating several model fields in one line of code?
just wonder why you all discuss that kind of shit: 'who's in charge', 'who should be punished', 'funny bug' etc in other topics.
This topic is better because it is about reality. protect your attrs blah blah
Here's the relevant section of the Rails manual describing this exploit, along with some ways to prevent it.<p><a href="http://guides.rubyonrails.org/security.html#mass-assignment" rel="nofollow">http://guides.rubyonrails.org/security.html#mass-assignment</a>
Regarding the argument between whether this is a Rails framework concern or a Rails developer's concern, the fact that many tutorials and screencasts don't bring this vulnerability up has left the impression to a whole slew of developers that scaffolding and other step-by-step out-of-the-box ways to build things that Rails affords you are complete solutions that don't require any other modifications besides what's needed for your own business logic, etc. I think this is how we all missed something so simple. I think it's partly because of this convention/idiom groupthink.
Seems strange that Active Record doesn't have an "update_fields" method with sig array of updateable fields, hash of field=>val. Similar to Sequel.
It's definitely the responsibility of the developer to know the security vulnerabilities and the guidelines of the tools he uses. Every serious frameworks have documentation on them and it is generally easy to find.<p>If the developer choose to ignore it, is the framework responsible of his action? I don't think so. Like other commenters have said, this is a beginner's mistake and they happen all the time. I don't understand how Rails can be blamed for this. They have done their duty by documenting the issue and it's easy to find (tell me who develops in Rails and is unaware of these guides?)<p>By the way, this feature is also known in Spring MVC (<a href="http://www.springsource.com/security/spring-mvc" rel="nofollow">http://www.springsource.com/security/spring-mvc</a>) and affect frameworks based on it too (ex: Grails). They state this is a "usage issue" and not a bug in the framework.
Totally avoidable via `Hash#slice`. Come on guys, this is common knowledge now.<p><pre><code> def update
find_pk.update_attributes(params[:public_key].slice(...))
end</code></pre>
Quickly formatted in a gist (only slightly tongue in cheek): <a href="https://gist.github.com/1975850" rel="nofollow">https://gist.github.com/1975850</a>
This has been Rails behavior from day 1. Rails seems to assume that people will make some level of effort to secure their application before deploying to production. There are many ways and places to protect models, and mass assignment protection is a blunt tool that would not have worked for github, so the default behavior is not the issue.<p>This bug could occur in any framework where someone assumed all attributes submitted are writable by the current user. Rails has no internal concept of users or roles, so building that by default into a model makes no sense.<p>This is a github bug, not a Rails issue. One could argue it's a questionable, but defensible, decision in the Rails framework, to have such an easy way to take every submitted field and apply it to a model. I'd argue that using such a feature in a production app is a fault of the developer for failing to read their own code, because it's rather obvious and clear what the code does:<p>@product.update_attributes(params[:product])<p>Does exactly what you'd expect it to do.
I find it really surprising that Rails is taking heat for this. "Protect your attributes" is something you learn really early on, and most of my models will have a spec along the lines of "as a user, I can't steal another user's _____"<p>On top of that, public facing code should be written like<p><pre><code> def update
@pk = current_user.public_keys.find(params[:id])
# do the update if you find the key
end
</code></pre>
Simple stuff.