Rails Commit: Whitelist all attribute assignment by default | Hacker News Reader

Rails Commit: Whitelist all attribute assignment by default

github.com

95 pointsricksta28 comments

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.

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.

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.

... and this stunt will cost Github five to six figures.

This is fascinating to me. It would be great to learn the reasons why it will cost them so much.

(I don't doubt it; quite the contrary. I'm inexperienced and would love the opportunity to learn more about the realities of the modern security industry.)

Sure. First and most important thing for understanding Real World costs: one engineer is $20k per month in fully loaded costs (salary, benefits, taxes, overhead). One engineer-day costs you $1k. These go up for emergency response because a) on-call b) experts are even more expensive than generic engineers.

Github's first response to this would be pushing a Big Red Button that would get 4+ engineers to devote their Sunday to this. That's $4,000, cash money. The predictable second step after the bleeding stops is to do a line-by-line audit of their entire code base. My guesstimate for Github is that that would cost north of 50 man days ($50k).

But wait, there's more! As a result of this compromise, Github is likely going to hire external security firms to pentest them and make process recommendations. The caliber of firm they would consider employing will cost, bare minimum, five figures. Cost goes up pretty rapidly.

But wait there's more! Github will, as a result of this incident, have a number of people close accounts today (totally measurable) and an unknown number avoid creating accounts in the future. LTV for SaaS customers very quickly becomes motivational numbers. A single company moving its repo from Github to an internal system because Github Let Anyone See Any Repo (+) could easily cost $5k+ in LTV, and that scales horizontally across their entire client base. Scaring your customers' PHBs is never fun. This issue will be held against Github in a thousand internal conversations.

But wait there's more! Highly visible security problems will bring Fortune 500 lawyers out to play. "You just caused a security audit for us. It cost $250,000. Where should we send the invoice? Oh, you think that your Terms of Use says you don't owe us? Cool, let's run that by Legal: they're free this week."

Long story short: getting hacked is Very Bad News.

+ This is the key takeaway from the hack, not "Someone did a one-line defacement of an OSS project."

> The predictable second step after the bleeding stops is to do a line-by-line audit of their entire code base.

And this would be unnecessary if a vulnerability was discovered internally rather than demonstrated to exist by a well-meaning outsider? (Never-mind a malicious third-party).

A bank that left the back door open wouldn't need to conduct an audit if the breeze was noticed a few years later by a teller, rather than if it was pointed out by a customer throwing notes on paper aeroplanes through the open door?

(Ditto all your other points).

You cannot and should not assume that a "0 day" (questionable terminology in this case) has not already been discovered and exploited.

" this stunt will cost Github five to six figures."

The vulnerability already existed. There could have been github customers' accounts getting hacked without the customer knowing and leaking confidential information. There can be easily more than five to six figures, borne by Github's customers here.

Rails is insecure by default and the website would have required work==salary anyway to be secured. Instead of fixing it for cheap early they have to fix it for several times that cost now. Since we all know time is money and interest is paid on loans, paying a higher price now is only a natural consequence of not fixing it earlier.

Therefore, I don't believe it is the stunt that costed github five to six figures. The loss of wealth was already there from day 1 when Github developers did not read Rails documentation and/or when Rails decided to make attributes publicly accessible by default. Today it is merely a "correction" where instead of Github's customer losing confidential company information without knowing it is now Github bearing the costs upfront, as it should be.

In the "emperor has no clothes" story would you say it was the kid who pointed out the emperor had no clothes caused the emperor's embarrassment? The emperor never had any clothes, he should be embarrassed, but he wasn't. The kid pointing it out corrected this, and I believe the same has happened here with Github, Rails and Egor Homakov.

    this stunt will cost Github five to six figure

It will cost its customers a lot more to reaudit all their commits from the date they migrated to GitHub. How can any one be certain at this stage that there hasn't been a 0-day inserted into their project?

How could anyone be certain that there hadn't been a 0-day inserted in their code, even if the method of disclosure had been different?

True.

This makes a strong case for requiring every git commit in general to be signed be the user's key.

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.

See below. 99% of the total cost isn't accrued yet.

> (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.

I agree with your points but what do you think could have been done to "convince" the Rails team to fix this? Short of the community abandoning/boycotting Rails, perhaps a revolution/spectacle was required.

This little exercise reminds us that the dysfunction seen inside corporations are just as easily present in open source communities. In the end, management holds the keys to what gets checked in and what doesn't.

The curious twist in this whole tale is that PHP- because of its history of being insecure- actually had a hardened distribution, while Rails didn't.

This type of smugness creates its own karma, as evidenced by the red-faced hack of a premier RoR site.

Thank you, @homakov

They should've at least thanked the guy.

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.

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

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.

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.

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

Lack of humility.

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

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

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

It's hardly just outright arrogance.

In other words:

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

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

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.

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.

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.

Reply on news.ycombinator.com