top | item 3663262

"Egor, stop hacking Github"

446 points| llambda | 14 years ago |homakov.blogspot.com

111 comments

order
[+] JangoSteve|14 years ago|reply
Given the recent story on HN about how former YouSendIt founder had taken their servers down to prove their vulnerability [1] [2], I'm surprised how little reverence these "lol-hackers" (that's going to be my term for them) give to showcasing these vulnerabilities by exploiting them in the real-world and messing with people's real things.

I know as hackers, we feel a duty to show people how serious these things are and that we get impatient and annoyed when ignored. And I also know that it's hard for us to reconcile the idea that when we show the owners rather than tell, it's suddenly considered a crime. But it is.

Let's try an analogy. Door locks on houses are ineffective. Think about it. Your house is covered with windows, which are made of glass. Glass is really easy to break. I mean really easy. If you found out your neighbor didn't have a house alarm, you might talk to them and tell them they should get one. If they didn't get one, would you then break into their house one night and walk into their bedroom to show them how dangerous it is?

OK, who knows, maybe you have a weird relationship with your neighbor. Furthermore, this is an imperfect analogy, because here, Rails and Github are both responsible for other people's property.

But now imagine it's a business across town and that you don't actually know the business owner. If you broke into their business to show them their building's security vulnerabilities, you bet your ass they would press charges and I don't think anyone would blame them. Even if you're doing it with the best intentions, it's still vandalism at best.

All of that being said, this is a very effective way of making your point and getting people to fix the problem. That doesn't make it right. But if you're willing to put yourself in harm's way and essentially become a martyr to get these security vulnerabilities fixed, more power to you.

[1] http://news.ycombinator.com/item?id=3643102

[2] http://www.inc.com/magazine/201203/burt-helm/a-silicon-valle...

[+] marshray|14 years ago|reply
Door locks on physical houses are just a small part of a much greater security picture involving your community, observant neighbors, other monitoring systems, the police, even dogs. And yes, houses do get broken into causing a massive amount of aggregate economic loss every year.

But on the internet everyone's front door is accessible from even the most remote and hostile places in the world and you're pretty much on your own for securing it. Internet-facing systems also tend hold far more valuable things than your typical home.

For these reasons, I don't think the household lock analogy works very well.

[+] sek|14 years ago|reply
This analogy is just wrong, on the web exists perfect anonymity. Very often you also can hide your traces and nobody will ever find out what you did.

You have to assume every exploit will be abused. The most stupid thing you can do is attack the white hats, because the next hacker will shove your stupidity down your throat. On a lulz way or on a black hat way.

I would never let my data near a company which doesn't respect white hats, for security and moral reasons. I would remind you that we are on Hacker News, that doesn't exclude the popular meaning of the word.

[+] ben0x539|14 years ago|reply
Breaking some poor dude's window probably causes a whole bunch more economic damage than a spurious commit into a github repos.
[+] wisty|14 years ago|reply
There's an old debate over what constitutes "gray hat" hacking, and whether it's moral.

I haven't looked at Egor's actions too closely, but he seems fairly close to a "white hat". He's been a bit immature, and so has Github, but there's been no real lasting damage.

Anyone who's written a web app knows that mistakes can happen, and they take time to fix. I'm sure Github will handle the PR crisis with an informative and unbiased mea culpa (even if they secretly think Egor was a bit brash). People will keep using them, because their interface rocks.

This will stay on the front page for about 24 hours, then there will be a few link bait articles that also cash in on it, but that's about it.

[+] ricardobeat|14 years ago|reply
Terrible analogy. More like someone's automatic door lock is opening by itself at random times of the day, and the owner doesn't believe it, so you leave a note on the inside. It's creepy, but not wrong.
[+] ktizo|14 years ago|reply
If he used a known vulnerability to break into github, then he didn't break in, he walked in.

Admittedly he did then do the online equivalent of pissing on the wall, so minus several million points for artistic endeavor.

[+] lnanek|14 years ago|reply
This reminds me of how PHP used to turn HTTP request variables directly into global programming variables by default. Now it only happens when you enable the register_globals option. I don't think I've ever met anyone who didn't consider it a huge security issue.

This rails behavior is actually even more powerful than the old PHP one for hackers because with this you get directly into the model and then the DB when everything is still left as generated, not just the temporary variables. It's actually pretty surprising how much resistance there is to fixing the issue.

It could be that the proposed whitelisting isn't the only solution. It does require annoying configuration. With PHP, nowadays, most people just access a particular array when they want their request variables. Similarly, maybe Rails could have a request model object and a DB model object with simple methods for copying state between the two. Maybe combine it into some sort of validation logic with user friendly error messages being specified. I guess it is still more work that default overwriting of the DB with request variables, though.

[+] Jach|14 years ago|reply
I was also thinking of PHP's register_globals. I was tempted to make a snide remark, so I'll make it now. The difference here is that the PHP group realized register_globals was a bad idea, deprecated it in 5.3 and removed it in 5.4. Furthermore the default has been OFF since 4.2.0. The resistance to fixing the Rails problem just makes me ever less likely to give Rails a shot, it should be really bad PR when you ignore security issues.
[+] qeorge|14 years ago|reply
I like the approach Yii (PHP framework) takes:

http://www.yiiframework.com/wiki/161/understanding-safe-vali...

If a field doesn't have any validation rules set it will be thrown out when you save the model. This way you won't mass-assign to a column that was never meant to be mutable. Its a little more work to get up and running, but I think its a good tradeoff.

(You do have the option of turning this off, but you'd have to do it intentionally).

[+] wycats|14 years ago|reply
For what it's worth, we'd like it if security vulnerabilities in Rails were disclosed to http://rubyonrails.org/security.

Obviously this situation is a bit more complicated, as a ticket was opened up, and a lot of community discussion occurred. In general, emails to the security list are taken extremely seriously.

[+] saurik|14 years ago|reply
Where would one find that link? The link to "Bugs/Patches" on rubyonrails.org links directly to the GitHub issue tracker. A search on Google for "link:rubyonrails.org/security" finds only a single reference, and it is from someone evaluating how other vendors handle security issues. (My guess: it used to be linked, but then Rails moved to GitHub, and some intermediate landing page was lost in the shuffle.)
[+] benatkin|14 years ago|reply
OK, here's a specific mistake made by a rails committer in handling this issue. drogus closed [this issue](https://github.com/rails/rails/issues/5239) without bothering to tell GitHub about it. This meant that fewer people saw it, and fewer people had a chance to tell GitHub about it. (One even thought of telling them about it, but unfortunately suggested that someone else do it instead of actually doing it.)

I think that perhaps the Rails team should have someone reviewing that issues were properly handled.

[+] boundlessdreamz|14 years ago|reply
People are reading this as a vulnerability in rails when it is actually a vulnerability in GH's code. Your comment unfortunately is going to add to it :(.

What should he have reported to rails security team? Shouldn't he have been contacting GH security team instead?

Edit: Rails guides discusses the root cause and counter measures against these type of vulnerabilities http://guides.rubyonrails.org/security.html#mass-assignment

[+] vidarh|14 years ago|reply
What is complicated about it? Someone finally called the Rails team out seriously on a long-standing well known security problem.

This is the type of ridiculous stuff we used to expect from PHP years ago.

[+] earl|14 years ago|reply
Well, Egor's hack accomplished two things:

(1) it has seemingly embarrassed some rails committers into taking this seriously, whereas they dismissed the issue before;

(2) I bet there were at least 20 devs who saw this on HN, said fuck my life, and hopped on their vpn to check if their site is vulnerable.

No drama disclosures didn't accomplish either of these things. Hopefully github won't take it too personally, and Egor was actually (as he seemed!) careful not to break anything.

[+] jclem|14 years ago|reply
Are people misunderstanding this?

It is not a bug. It's an acknowledged part of the framework (http://guides.rubyonrails.org/security.html#mass-assignment), although one that could get a developer into trouble without knowing to protect attributes where necessary.

If he'd known that there was something the GH team missed, he should have just brought the issue to them directly. If I realize my neighbor's house is in danger of collapsing because the contractor used the wrong type of wood, I don't bring the issue up with a lumber yard and then knock over my neighbor's house to prove a point when they ignore me.

I agree that it's a problem that many developers aren't aware that they need to protect against mass assignment, but it seems like this dude is totally misunderstanding the entire ecosystem here, and now people are calling him a "hero" because he took advantage of something that everyone already knows.

Big whoop.

[+] maratd|14 years ago|reply
> If I realize my neighbor's house is in danger of collapsing because the contractor used the wrong type of wood, I don't bring the issue up with a lumber yard

Yes, you do. Because the lumber yard sold your neighbor the wrong wood.

If you're going to make a framework for a language, do so in a manner that discourages stupidity.

PHP learned this the hard way quite a few years back. You never make things easier for the end-user at the expense of security. It's time for Rails to learn the same lesson.

[+] ben0x539|14 years ago|reply
A harmless, spurious commit is hardly comparable to the insane property damage of destroying someone's house, likely to bankrupt a private person.
[+] waffle_ss|14 years ago|reply
If this vulnerability is really due to attr_accessible, then that's got to sting for GitHub as this is a well-known "insecure by default" issue in Rails, and GitHub is probably the most (or one of the most) public Rails app out there.

Would be kind of scary if he had injected some nastiness into the rvm repo master branch, for instance, because I know some people do ride on the master version (`rvm get head`). Or, some gems that are built from Gemfile's pointing at the git repo on GitHub.

Luckily, git itself is quite resilient to attacks on the repo integrity so I don't think there could be much long-term harm done (no rewriting repo history would go unnoticed, for example).

[+] nchuhoai|14 years ago|reply
Can't decide whether to love or hate this guy. For his young age, he seems to have an impressive skill set, but you can totally tell the douche growing inside
[+] homakov|14 years ago|reply
I 'm not that douche you want to show me. But it seems so, sorry.
[+] Bootvis|14 years ago|reply
OTOH he didn't cause any real harm and disclosed the bug almost immediately. Don't be too harsh on the guy.
[+] etcet|14 years ago|reply
You must not have been your average 18 year old. This guy had read/write to GitHub and potentially many more sites, imagine what damage he could have caused. He may not have done it perfectly but he Did the Right Thing(tm).
[+] nchuhoai|14 years ago|reply
I'm not saying he is a douche for breaking into github. its more like how he acts and what he says:

"Then, I could make a post pretending i am DHH. That was funny too."

"GH sorry, I was bored."

Im sure everyone has different definitions, for me its just someone that I wouldn't wanna hangout with on a personal level, that's all.

[+] 16s|14 years ago|reply
In some areas, what he did is illegal and he could be prosecuted for it. I hope that does not happen, but if I were him I would be cautious.
[+] gaius|14 years ago|reply
'Bout time someone took the Rails "rockstar ninjas" down a peg or two.
[+] pjscott|14 years ago|reply
Have they wronged you in some way?
[+] javajosh|14 years ago|reply
This is not a rails bug, but a github bug. The discoverer reported it in the wrong place, the rails people (correctly) told him to report it to github, he ignored them, and this hilarity ensued.

While this is a legit bug on github, and Egor deserves credit for finding it, he also deserves a scolding for the classic noob mistake of not reporting it to the right place.

[+] abalone|14 years ago|reply
That's a matter of opinion. He intentionally reported it against rails because he thinks it is rails' fault for making it so easy to have this security problem. Given that the #1 rails app out there written by professionals suffers from it, and reportedly others do too, he has a point. You can disagree if you want but it's definitely not a "classic noob mistake".
[+] javascriptlol|14 years ago|reply
There is no reason to be introducing "fail open" design flaws at any level above the bare metal.
[+] rmoriz|14 years ago|reply
sometimes it's better to do bold moves when some nastiy thing needs attention and noone is really hurt.
[+] joelhaasnoot|14 years ago|reply
Dont quite think this is what I'd call "responsible disclosure", and not exactly sure this is "disclosure" either.
[+] oconnore|14 years ago|reply
He made an attempt to patch upstream, was shot down, and then proved his point without hurting anyone. Maybe not the best way to get the issue out, but we are talking about it and it wasn't especially harmful. Mission accomplished?

I don't know how else he could have done this. He was being ingored!

[+] mrsteveman1|14 years ago|reply
The linked post says "Why I do this? Since guys in rails issues ingored me".

Sounds like this was already a known problem that has led developers to shoot themselves in the foot.