Live data from Hacker News

Hacked: commit to rails master on GitHub

github.com

61–70 of 240 posts

Re: Hacked: commit to rails master on GitHub

#61

Earlier quoted context omitted.

The issue seems to be a result of an entire hash of the parameters passed in the request being sent to the new method for models. Is this really common practice in Rails code? Sure you can specify in the model that certain attributes can't be changed. But shouldn't this stuff be checked when validating form input? Normally I'd have a hash of filters, with the field/column name mapped to the appropriate set of rules f…

It's common in tutorials and quickstart guides, usually with a note saying that you really should be protecting things and not using mass-assignment. It just makes the blog in 5 minutes videos cleaner.

Reminds me of all the PHP tutorials floating around that only treat security as a side note. Essentially, the default configuration with Rails seems to be that users can set the data for any database column. It's almost as bad as register_globals...

Re: Hacked: commit to rails master on GitHub

#62

Earlier quoted context omitted.

Regardless of the 'hacker's motives/personality, I think this is yet another testimony to the power of open source. When you have this many eyeballs looking at your code, the odds of a good-intentioned (however playful/immature) coder to discover a vulnerability is much greater than those of a real ill-intentioned hacker simply due to the sheer number of the former. The issue will quickly get fixed by the community,…

Hmm? The vulnerability is in github's code (not open source) and not in rails.

The vulnerability is that Rails is insecure by default. That used to be the case for a lot of things, then finally people noticed how the real world works, and started fixing them. Apparently the Rails developers have actively resisted the lesson everyone else already learned.

Re: Hacked: commit to rails master on GitHub

#63
post #18

Earlier quoted context omitted.

This guy brought up the vulnerability and the maintainers didn't seem to take it seriously since he wasn't articulate enough or was not approaching them with enough respect maybe for their liking? I wish they would have kissed his ass a little to get the low-down on the vulnerability so I didn't have to worry about my company's private github repos. He deserves props for bring it up for discussion.

This has been discussed since the early days of Rails, and they have chosen to leave their defaults as such and encourage developers to implement model security as needed. Github (seemingly) did not implement model security. This is a vulnerability that is different from application to application, and if the team was following best practices, is not there.

And he's just demonstrated why this is not a responsible approach from the Rails team.

Re: Hacked: commit to rails master on GitHub

#64
post #53
post #14

Earlier quoted context omitted.

Relevant: What I want you to see in that thread I mentioned is the way the core team perceives this. You are not discovering anything unknown, we already know this stuff and we like attr protection to work the way it is. Looks like this guy got really frustrated with the Rails devs basically saying that he didn't know what he was talking about. This reminds me of all of the unsafe defaults that PHP used to have. Same…

This justification seems especially odd to me since Rails did so much in the first place to popularize the idea that the default behavior should be the one most likely to be "right". Don't they (or didn't they at one time) have a mantra "convention over configuration"? I guess that doesn't apply to security.

This also makes it all the more serious. As the PHP developers found out the hard way:

When you make it really easy to get started, a lot of people won't learn the system in depth enough to understand all the issues because they don't need to in order to make it work "well enough" for most cases.

By making Rails so easy to get started, they pretty much guarantee that there's going to be a ton of developers that don't pick up on, or forget, that they need to deal with issues like this.

That even a site like Github was vulnerable to this demonstrates just how seriously wrong it is to pick a default like this..

Re: Hacked: commit to rails master on GitHub

#65
post #60
post #44

Here's my proposal for improving the situation: https://gist.github.com/1974187 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' conventi…

> Merb's approach was to have mass assignment protection in the controller, and I personally think it's self-evident that it belongs there. But wasn't Merb merged into Rails? :) Sigh...

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.

Re: Hacked: commit to rails master on GitHub

#67
This is clearly a problem: with Rails' approach being 'right from the start', having no protection by default is not the right way to do it. This issue may be well known among the type of people that use github and read HN, but if someone had read about Rails being an awesome framework to make db-driven websites, they might not be aware of such a thing as a "mass assignment vulnerability."

If by adding a line or 2 to the code for generators can stop this, even if it includes a comment saying "Removing this line will do x y z", then I think the rails team could've treated the bug with a little more respect.

As @ericb said, if strong devs make this mistake, there's something wrong with the code.

I think it should also be noted that he didn't do anything malicious like trash repos, and even says on his blog:

    "Then I could wipe any post in any project. That wasn't that funny but pretty
    dangereous[sic]. It got more curious."
All he did was add a 3 line file to the master repo of a project that he was frustrated with. It generated all this attention, and will probably make them rethink the approach...

Finally: big props to the GitHub team for patching their vulnerability in <1hr on a Sunday...

Re: Hacked: commit to rails master on GitHub

#68
post #15

Earlier quoted context omitted.

One of the first comments is a link to where he tried, but the issue was repeatedly closed.

He submitted it to… the rails bug tracker. Three days ago. That's not disclosing the issue to GitHub at all .

It's a pretty insane leap of logic to maliciously attack the Github website to prove your point regarding a framework hosted on that website. Politely contacting people about the vulnerability, rather than pulling that crap in an extremely public venue seems like the more mature, and less incredibly illegal way to go about things.
Post reply on HN