

Rails Commit: Whitelist all attribute assignment by default - ricksta
https://github.com/rails/rails/commit/06a3a8a458e70c1b6531ac53c57a302b162fd736

======
patio11
I don't think hacking sites to do security activism is a good idea, at all.
However, many people over a period of years said that this change needed to be
made, in all of the friendly, ethical, polite OSS-approved ways. Tickets, blog
posts, emails to security, etc etc. I got to the issue a few years late, saw
the WillNotFix tickets, and wrote up - I kid you not - an on-dead-tree
_journal article_ which said:

 _[W]rite access to sensitive data should be limited to the maximum extent
practical. A suitable first step would be to disable mass assignment, which
should always be turned off in a public-facing Rails app. The Rails team
presumably keeps mass assignment on by default because it saves many lines of
code and makes the 15-minute blog demo nicer, but it is a security hole in
virtually all applications._

<http://queue.acm.org/detail.cfm?id=1964843>

This doesn't justify the Bad Guys abusing third-party sites with this, but the
Good Guys did everything right and did not achieve a fix as a result of doing
so.

~~~
sc00ter
AKA "The end justifies the means". It wasn't ideal, but it achieved and end
result that others have been asking for for years to no avail. That justifies
the means in my book, and then some. I hope Egor will be given sufficient
indemnity for his actions, as, even if misguided, they were clearly well
intentioned.

~~~
patio11
I strongly disagree both in general and as regards web vulnerabilities. For
one, Github is in no way responsible for the decisions of the Rails project,
and this stunt will cost Github five to six figures. (Anyone who thinks this
is in any way exaggerated has not been on an Incident Response team or read
what has to happen in industry. Heck, there are individual _customers_ of
Github who may have to push the Big Red Button right now.)

For another, there exist many, many applications against which one could
demonstrate a bug in Rails or Ubuntu or SSL or just the finding "Lol, P=NP."
Pragmatically, would you like that to be your application? I would very, very
much not like someone to do one of these to my sites. It would cause an
instant nightmare for me. The _happy_ outcome is I lose thousands of dollars
and don't sleep for most of a week. The unhappy outcomes are not upper-bounded
by death of my business.

n.b. Busting into Basecamp to make a point about Rails would also be morally
irresponsible.

~~~
oconnore
How would you be losing sleep now? Assuming you were Github, as of now you
would have (1) identified the problem, (2) fixed the issue, (3) handled the
public relations with a press release that includes a future disclosure
policy. You probably blew some cash on overtime pay this weekend, but how do
you count 5-6 figures?

Any company that is paranoid enough to cut their Github contract following a
benign intrusion that was resolved in under 24 hours is not a company that
would have uploaded code in the first place. This is not the only security
vulnerability on Github.

I think you are being excessively dramatic.

~~~
maratd
> (2) fixed the issue

They were notified of the issue 3 days ago. Today, he simply found another
instance of the very same problem in a different place. The vulnerability is
throughout their entire code-base.

They are not even close to resolving this.

------
hasanove
Thank you, @homakov

------
dmishe
They should've at least thanked the guy.

~~~
ricksta
I actually had "Thanks to Homakov" inside the title of this post but I guess I
wasn't allowed to have that in the title and someone changed it.

~~~
dools
Yeah there's a guideline on "editorialising in the title"

------
oomkiller
It still auto-adds the attr_accessible list which is nearly as bad as allowing
mass assignment in the first place. At most it should add an informative
comment, and the error message about being unable to mass assign attributes
should be made better, possibly with a URL to the existing mass assignment
Rails guide.

------
dos1
Good for Egor Homakov. I don't necessarily agree with his methods in this
case, but in the end, the net result of his actions was positive for web
security. And as so many others have pointed out, when the proper channels
aren't working, sometimes a little spectacle is just what you need to
instigate change.

In other news, I'm not sure why the rails core team was so against this in the
first place. Making things safe by default is usually a good idea. This change
doesn't seem to add too much additional ceremony, and people had been asking
for this for some time.

~~~
teyc

       I'm not sure why the rails core team was so against this in the first place.
    

Lack of humility.

~~~
Volpe
That's a bit harsh, it is at least a grey area.

Mass-assignment by-default, with disabling fields you want to protect

vs

Mass-assignment off by default, with enabling fields you want to mass-assign.

It's hardly just outright arrogance.

~~~
Peaker
In other words:

Security vulnerabilities by default, with the ability to patch the
vulnerability on a project-by-project basis.

vs

Security by default, with the ability to allow potentially insecure assignment
on fields you want to assign.

~~~
rapind
This seems like an intentional oversimplification. Having mass-assignment on
by default and therefore requiring the developer to create a whitelist of
attributes for each of their models has a complexity cost associated with it.
Failing to mention this at all shows a lack of objectivity.

I happen to believe it's worth it for the increased security, however I'm not
going to pretend that there isn't an associated cost.

~~~
teyc
It is 2012, we should no longer have to debate about the acceptability of
unvalidated inputs in a web application. Even PHP ditched register globals
long ago.

------
javascriptlol
Unfortunately people never learn from the past. Security holes directly
traceable back to the design of C are still trickling out after 30 years. The
entire construction of the web has failed to learn from this lesson. The whole
design is "fail open", and one mistake ends with site credentials being dumped
on pastebin.

