
"Egor, stop hacking Github" - llambda
http://homakov.blogspot.com/2012/03/egor-stop-hacking-gh.html
======
JangoSteve
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...](http://www.inc.com/magazine/201203/burt-helm/a-silicon-valley-tale-
of-humiliation-and-revenge.html)

~~~
marshray
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.

~~~
JangoSteve
EDIT 2: Hmm, maybe I shouldn't have removed my original response, I just
didn't want to derail the conversation upon second thought. For posterity's
sake, my original response was something along the lines of this:

\---

I think you may be overestimating the effects of "the greater security
picture" involving community, observant neighbors, other monitoring systems
for the average community. For example, my first startup was
RateMyStudentRental.com, and as I recall adding automated motion-activated
sensors and lighting actually had a negative effect on break-in protection. As
in, adding such a system actually increased your chances of having a break-in.
According to the study, neighbors don't actually pay much attention to someone
standing at a well-lit door (mileage varying based on the specific community
obviously).

\---

ORIGINAL EDIT: I won't comment on the technicality of the differences between
the analogous situation and the actual situation at hand; the differences have
nothing to do with the point being made. Don't get caught up in the analogy,
because it's just an analogy.

In case it wasn't clear the first time around, the entire point of the analogy
was that, breaking into and vandalizing someone's business just to show them
that it can be done is generally a bad idea. Though I don't think anyone would
argue it isn't effective.

~~~
humbledrone
_> [...] automated lighting systems actually have a negative effect on break-
in protection [...]_

Do you have a source for this? I've always been under the commonly held
impression that this has a preventative effect, and it would be very
interesting if the opposite really is true.

~~~
JangoSteve
Good question. It was a long time ago, and I'm having trouble googling for it,
because all I get in search results are articles and how-tos by motion-sensor
companies selling their products. I'll search though and update if I find it.

UPDATE: Hey, awesome, turns out we blogged about it back in the day:
[http://blog.ratemystudentrental.com/2008/03/05/a-safe-
home-i...](http://blog.ratemystudentrental.com/2008/03/05/a-safe-home-is-a-
happy-home-well-it-helps-anyway/)

Here is the original article:
[http://www.schneier.com/blog/archives/2007/09/light_and_crim...](http://www.schneier.com/blog/archives/2007/09/light_and_crime.html)

 _Much so-called security lighting is designed with little thought for how
eyes -- or criminals -- operate. Marcus Felson, a professor at the School of
Criminal Justice at Rutgers University, has concluded that lighting is
effective in preventing crime mainly if it enables people to notice criminal
activity as it's taking place, and if it doesn't help criminals to see what
they're doing. Bright, unshielded floodlights -- one of the most common types
of outdoor security lighting in the country -- often fail on both counts, as
do all-night lights installed on isolated structures or on parts of buildings
that can't be observed by passersby (such as back doors). A burglar who is
forced to use a flashlight, or whose movement triggers a security light
controlled by an infrared motion sensor, is much more likely to be spotted
than one whose presence is masked by the blinding glare of a poorly placed
metal halide "wall pack." In the early seventies, the public-school system in
San Antonio, Texas, began leaving many of its school buildings, parking lots,
and other property dark at night and found that the no-lights policy not only
reduced energy costs but also dramatically cut vandalism._

I remembered it incorrectly, as apparently motion-activated lighting helps,
but on-all-night floodlighting makes the situation worse.

~~~
marshray
Peter Gutmann reported some interesting studies in his keynote talk at
Shmoocon this year. It's probably available on Ustream or the Shmoocon site.

In short, sometimes darkness helps. Consider a tall glass office building. If
all the lights are out, nothing stands out to a security guard more than a
flashlight waving around in the darkness.

------
lnanek
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
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.

~~~
kijin
Welcome to the new decade, where PHP has secure defaults and Rails apps get
pwn'd left right and center.

But seriously, PHP still has lots of problems to fix.

~~~
maratd
> But seriously, PHP still has lots of problems to fix.

Like what?

~~~
getsat
<http://phpsadness.com/>

<http://www.reddit.com/r/lolphp/>

Among others...

~~~
maratd
If the best you can do is close to a year stale and doesn't apply to the
latest version, I'd say PHP is doing a fine job.

Also, reddit? Really?

~~~
masklinn
> If the best you can do is close to a year stale and doesn't apply to the
> latest version

Really? 5.4 has fixed the retarded associativity of the ternary and all error
reporting?

------
wycats
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.

~~~
boundlessdreamz
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>

~~~
dhconnelly
When I read "this vulnerability affects tons of Rails apps," I read that as a
security vulnerability in Rails. I'm not a user of the framework, but I've
heard "convention over configuration" often enough to think that if this was
brought up in the Rails issues tracker, it should be prevented by convention.

~~~
marshray
If it affects "Hello Rails" the canonical example Rails application, then
maybe it is arguably a vulnerability in Rails.

[http://guides.rubyonrails.org/getting_started.html#say-
hello...](http://guides.rubyonrails.org/getting_started.html#say-hello-rails)

------
jclem
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
> 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.

~~~
Drbble
You can't chop off half of someone's sentence and then debate the leftover
part. Not at an "and", at least.

------
waffle_ss
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
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
I 'm not that douche you want to show me. But it seems so, sorry.

~~~
marshray
Dude, you're a hero.

Others just don't realize it yet because they're going through the stages of
grief (Denial, Anger, Bargaining, Depression, Acceptance).
<https://en.wikipedia.org/wiki/K%C3%BCbler-Ross_model>

If no one else says it, I will:

Thank You!

------
gaius
'Bout time someone took the Rails "rockstar ninjas" down a peg or two.

~~~
pjscott
Have they wronged you in some way?

~~~
click170
Some people are jealous of success.

~~~
to3m
Rather, note tenor of comments at
<http://news.ycombinator.com/item?id=3651958> .

------
javajosh
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
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".

------
BenjaminCoe
Alright, this did a pretty good job of winning me over to Egor:

<http://homakov.blogspot.com/2011/07/octocat-tattoo.html>

That tattoo is legit.

~~~
skeletonjelly
Maybe not?
[https://github.com/rails/rails/commit/b83965785db1eec019edf1...](https://github.com/rails/rails/commit/b83965785db1eec019edf1fc272b1aa393e6dc57#commitcomment-1041295)

------
rmoriz
sometimes it's better to do bold moves when some nastiy thing needs attention
and noone is really hurt.

------
joelhaasnoot
Dont quite think this is what I'd call "responsible disclosure", and not
exactly sure this is "disclosure" either.

~~~
oconnore
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_!

~~~
stock_toaster
> He made an attempt to patch upstream, was shot down, and then proved his
> point without hurting anyone.

Assuming no economic impact to github you mean. If paying users leave because
they feel github is no longer safe for private repos because of this, that is
harm.

I imagine a bank would be far more grouchy if someone exploited a
vulnerability and deposited one cent into someone's account "just to show
there was a vulnerability" then publicized it, without talking to them about
it first. <sarcasm>No harm done right?</sarcasm>

Now, I am not saying that it is bad that this github vulnerability was found
and fixed. I am very glad! But I think it could have been far more responsibly
done.

~~~
Drbble
If github users quit because a hacker deletes their stuff, maybe that's harm
the hacker did to github.

But if github users quit because a hacker shows them github is insecure,
that's harm github did to itself. Don't blame the messenger.

------
Volpe
tl;dr; - Learn about: attr_protected[1]

[1] <http://guides.rubyonrails.org/security.html#mass-assignment>

