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.
Hacked: commit to rails master on GitHub
61–70 of 240 posts
Re: Hacked: commit to rails master on GitHub
#62Earlier 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.
Re: Hacked: commit to rails master on GitHub
#63Earlier 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.
Re: Hacked: commit to rails master on GitHub
#64Earlier 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.
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
#65Here'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...
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
#66We've patched and fixed this on GitHub.
Re: Hacked: commit to rails master on GitHub
#67If 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
#68Earlier 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 .
Re: Hacked: commit to rails master on GitHub
#69Re: Hacked: commit to rails master on GitHub
#70 "Discount for girls"