Posting 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 responsible approach.
That might be unprofessional but it was also audacious - and easily the best way to get the word out quickly about an 'in the wild' exploit that will impact a huge number of apps.
Additionally: GitHub is a critical piece of infrastructure for a huge number of companies, contractors and start-ups. On balance, protecting GitHub from public embarrassment is far outweighed by the potentional impact of this sort of flaw. If a good old public "git@github ~ $ rake over coals" is what is needed to ensure that our IP is sufficiently protected, so be it.
Egor discovered that a lot of big sites which use Rails suffer from very serious security issues because the common Rails practices and defaults don't produce secure sites. It's so non-obvious that he was able to impersonate others, avoid most of the checks...
He was aware of consequences, but he got mostly ignored. So he decided he had to get enough attention and publicity. There are too many sites too vulnerable to just wait.
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.
Exactly. The guy might not have the best English or highest level of maturity, but it's not like he found a flaw and ran around saying, "Haha, look what I can do!" with no justification.
He submitted a security flaw to the Rails issue tracker. It was shut down by committers saying, "This isn't a real flaw, it's everyone's responsibility to secure their own apps."
At that point, a reasonable response is, "Yes it is, you dummies. Watch as I use it to pwn multiple high-profile production rails sites."
Fred Wu
So what's a good gem (if any) for safe guarding params on the controller level? @dhh's params.slice feels too dirty.
DHH
yeah, it's too simple to be clean! I think you are using the wrong framework if you crave more complexity for its own sake.
The users who trust Github don't care if it was a "rails issue" or a "github issue"
It's a Github issue no matter what the cause or responsible party.
If Github was run on unpatched windows XP boxes with all IPs public, you wouldn't argue that it was Microsoft's fault because "everyone knows" what a bad idea that would be. The base assumption should not to be completely reliant on your environment and frameworks to be secure, because they very likely have un-patched bugs and exploits. You do what you can to harden yourself up, but when something goes wrong - it's your problem.
But (to continue your analogy), github was run on fully patched XP boxes, in which case: yes, it would have been Microsoft's fault.
And it would have ultimately been Github's fault (in the eyes of their users) in that case too -- because they are providing the service, and their choices (XP / Rails, Firewall / Application Firewall / Audit) is their fault.
Contrary to how a lot of things are handled today, fault can be shared by multiple parties. what Egor did was get exposure and reaction from a lot of involved parties, where he was unable to get any reaction from some of them (Rails team) in the past.
Well, it's a beginner-level Rails mistake; it is not precisely an obscure issue. Googling 'attr_accessible' will show you discussion going back years, and it has been actively exploited before. I'm shocked that the mistake was made in Github, though.
"Well, it's a beginner-level PHP mistake; it is not precisely an obscure issue. Googling 'SQL Injection/register_globals/get_magic_quotes/etc' will show you discussion going back years, and it has been actively exploited before. I'm shocked that the mistake was made in ..., though."
Ten+ years of register_globals, get_magic_quotes and SQL injection attacks (etc etc etc) in PHP show that even well-known issues still bite people everyday. IMO, it's up to the framework developers to make it easy to do the right thing and damn hard to do the wrong thing.
I don't think it's on the same scale as register_globals or magic_quotes; both of those were much stupider. It's still very stupid, granted, but not on the same scale as register_globals.
Obviously not beginner-level if more famous sites suffer from the same problem -- Egor mentions some in his posts and he is only one person -- imagine what all the black hats can do working in parallel.
By your reasoning, since tons of C programs can't use strcpy() correctly and thus contain buffer overflows, it takes a C master to use strcpy() correctly.
I'd say instead that the kind of reasoning is wrong and that even experts can make beginner-level mistakes (not so often). But these are the kind of mistakes a good framework/language/etc. should try to prevent - especially if security-related. Tons of research tries to reduce error-prone programming activities.
My understanding is that it's an insecure built-in default. Ie, the mistake is by the Rails framework developers, and Rails users have to explicitly secure it. Is that incorrect?
IMHO, it's a bad practice which is very widely promoted in Rails guides and scaffolding code.
Mass-assignment is very difficult to get right in any complex app (e.g. with multiple user roles) and the simplest way to avoid these problems would be to discourage the use of 'update_attributes' and teach people to explicitly set model properties.
(Of course, Rails folks probably hate the non-DRY aspect of this, but I prefer to look at my code and see exactly what's happening with user input.)
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.
I'd agree with it being "not the most responsible" way if you were talking about him publicly posting an exploit instead of privately talking to github. But the vulnerability was already known, he didn't disclose anything new, so I don't see the lack of responsibility.
Fact is, if he hand't exploited the vulnerability in a high profile site, the extent of the vulnerability would be unknown and it wouldn't have been fixed. Since (damage done) << (benefit) i think we should be thanking this guy.
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 responsible approach.