This sounds like a "register_globals" feature. Are people still that obtuse to basic security?
Hacked: commit to rails master on GitHub
131–140 of 240 posts
Re: Hacked: commit to rails master on GitHub
#132IMO this attitude of GitHub is the best motivation to sell 0day exploits in private instead of ever trying to get dev's attention.
No, I mean really, "malicious attack". I can't help but laugh, he committed 3 lines of text grand total, this is what you call malicious? Seriously, WTF.
The guy is a proper white-hat hacker, even if somewhat childish, Y U ban him.
Re: Hacked: commit to rails master on GitHub
#133Everyone might as well take this opportunity to add attr_accessible to your models. Models: find app/models -type f -name \*.rb | wc -l Models with attr_accessible: grep -r -m1 "attr_accessible" app/models | wc -l If those numbers aren't the same, and the missing model files inherit from ActiveRecord::Base, then look into adding attr_accessible.
We included a spec in our rails app that inspects all AR models and errors if any don't specify any attr_accessible (with a whitelist of models to ignore). This catches anything included by plugins too, which can be helpful.
https://github.com/instructure/canvas-lms/blob/9b52a51b6a37e...
Re: Hacked: commit to rails master on GitHub
#134We've patched and fixed this on GitHub.
You've apparently also suspended his account...? ( http://homakov.blogspot.com/2012/03/im-disappoint-github.htm... ) If this is true, would you please consider unsuspending him? This doesn't seem like a good way to reward this sort of behavior (i.e., helping through hacking).
He clearly violated their Terms Of Service:
5. You agree not to reproduce, duplicate, copy, sell, resell or exploit any portion of the Service, use of the Service, or access to the Service without the express written permission by GitHub.
You do something dumb like this, you should live with the consequences. In legal terms, if they don't enforce their TOS, it can be a legal issue later.
I love how he punts on the terms with "but lets get real".
Analogy: You break into a house and get caught. "Oh yeah, laws. But lets get real, I was pointing out they didn't lock their door."
Re: Hacked: commit to rails master on GitHub
#135We've patched and fixed this on GitHub.
Thank you commenter "holman" on this third party discussion site, but as a paying customer could github please post something official somewhere like now
Re: Hacked: commit to rails master on GitHub
#136Earlier quoted context omitted.
You've apparently also suspended his account...? ( http://homakov.blogspot.com/2012/03/im-disappoint-github.htm... ) If this is true, would you please consider unsuspending him? This doesn't seem like a good way to reward this sort of behavior (i.e., helping through hacking).
He clearly violated their Terms Of Service: 5. You agree not to reproduce, duplicate, copy, sell, resell or exploit any portion of the Service, use of the Service, or access to the Service without the express written permission by GitHub. You do something dumb like this, you should live with the consequences. In legal terms, if they don't enforce their TOS, it can be a legal issue later. I love how he punts on the te…
He didn't exploit anything, technically--that's all in the service API. (thanks Rails!)
More seriously, just because he violated the ToS doesn't mean that they have to be dicks.
Re: Hacked: commit to rails master on GitHub
#137Earlier quoted context omitted.
The point, though, is that this is more of a Rails issue than a Github issue. If Github gets it wrong, others are likely to.
It is a Github issue in the fact that they didn't protect against this issue when they easily could have.
Others have made the comparison to PHP's `register_globals`.
Yes, Github had vulnerable code. It's also true that Rails apparently defaults to leaving that bit of code vulnerable. A saner default seems in order, if even highly competent Rails devs can be caught by this.
Re: Hacked: commit to rails master on GitHub
#138Here'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…
I like this proposed solution quite a bit. Currently, I like to use a hack that automatically makes all models use attr_accessible with no allowed attributes. Until you override it with attr_accessible in your model, nothing is allowed, so you have to think hard about what should and shouldn't be accessible. However, it causes real annoyance when not dealing with web input. Applying it to an existing project and fixi…
Right, refactoring an existing project to enforce attr_accessible nil by default can be a hassle.
From my experience, the main (though easily side-stepped) annoyance is when creating or updating records that have belongs_to associations (for example, user_id and repository_id for a commit ;)) programmatically.
For security purposes, you would not set those 2 attributes to be attr_accessible. To create a new record, you then would have to build the record and then set the user_id and repository_id on the record.
Or, you can set user and repository to be accessible (attr_accessible :user, :repository). This is fine because the associated methods expect ActiveRecord objects.
Re: Hacked: commit to rails master on GitHub
#139Earlier quoted context omitted.
> Responsible disclosure If you look at the bug report, the core Rails Dev Team basically said that they like the defaults the way that they are. They have/had no intention of changing the defaults, and are trying to push responsibility on to the developers using Rails to use sane config settings. Looks like the guy did report it and the response was: "Not our problem" / "Not an issue." He got frustrated and decided…
The Rails team essentially argued that it isn't on them to secure sites built with Rails. Rails provides tools to avoid this, and GitHub chose to not use them, so this is a vulnerability in GitHub. They should be using attr_accessible, and they're not.
The problem with this attitude is that such caveats are not mentioned where it would be most prudent to do so. Which means you have scaffolding and documentation that shows you can use mass assignment, but totally fails to mention what you also need to do if you're going to put that code into production. Instead, it's hidden in a totally separate section in the guides.
The approach this encourages is to code insecure (because you don't know any better), without any awareness of it being insecure, then remember to go back and secure it when you've found out what all the vulnerabilities are. Inevitably, you will waste time, and probably miss things out.
Re: Hacked: commit to rails master on GitHub
#140Posting it as an issue on the Rails repo and then exploiting GitHub with it is a great way to get attention, but not necessarily the most responsible. I disclosed a vulnerability to GitHub before. I dropped it into their Issues system marked private with the heading "URGENT". It was a Sunday and I got a response + a fix from Tom Preston-Wener himself within a few hours. That, in my mind, would have been a more respon…
I don't think he can be blamed too much though. As per the bug filed here - https://github.com/rails/rails/issues/5228, the bug was being closed by others after being given a cursory look, and was being reopened again for consideration. Maybe a little immature, but there was a mild provocation.