Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

How Homakov hacked GitHub & the line of code that could have prevented it

gist.github.com

Post reply on HN
Print view View original

41–50 of 91 posts

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#41

Post by baq »

Peaker wrote: post #27
jgrahamc wrote: post #3

It's surprising to me (not a very experienced Rails developer) that the default behavior should be so open to abuse. Why isn't the default the opposite?

Because convenience is deemed more important than safety. Same reason that "enum" and "int" are pretty much interchangeable in C, that arithmetic conversions and truncations are implicit -- it's not very safe, but it is more convenient.

you know, i've heard that from php. it was damn convenient in version 3.

we all know how it ended up.

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#42

Post by o1iver »

gcampbell wrote: post #34
o1iver wrote: post #25

"Homakov PUT an update to his own existing public key which included a new user_id. The user_id he used was that of a member of the Rails repository members." But wouldn't that mean, that the commit would display the username of the user with the `user_id' that he used?

Github displays the username/avatar for a commit based on that commit's Author field, not the user that pushed the changes to the repository.

Ok, I see. Thanks!

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#43

Post by o1iver »

judofyr wrote: post #35
o1iver wrote: post #25

"Homakov PUT an update to his own existing public key which included a new user_id. The user_id he used was that of a member of the Rails repository members." But wouldn't that mean, that the commit would display the username of the user with the `user_id' that he used?

GitHub uses the email-field in the commit to connect a user to a commit.

Ok, I see. Cheers!

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#44

Post by rcxdude »

Estragon wrote: post #17

Since this vulnerability has been known about for years and experienced crackers have presumably checked for it before, is there any reason to be concerned about more malicious corruption of other repositories on github?

it would still be pretty difficult to insert malicious code into a git repo, you would need to add a new commit to the end without being noticed. modifying earlier commits would be possible but git would complain when someone pulled from it in order to update a repository because the hashes wouldn't match up. New clones would work fine though.

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#45

Post by danso »

Estragon wrote: post #38
phamilton wrote: post #26

Earlier quoted context omitted.

The problems with update_attributes have been known, and messing with things like timestamps probably has been done before, but Rails itself didn't violate any acls. All users still can only access their own resources via Rails. This attack was a combination of Rails and git/ssh keys. There's a bit of a clever aspect to it, one that isn't implied merely by understanding the vulnerabilities of update_attributes. While…

OK, but that doesn't really answer my question, which is why people aren't more concerned about earlier more surreptitious corruptions of the repositories of github via the same vulnerability. It's got to have been an attractive target for the likes of the Operation Aurora [1] folks. [1] http://en.wikipedia.org/wiki/Operation_Aurora

Presumably, Github could check for this activity by finding all public key submissions in which a public key registration involved a user id that is not the same user id as the signed-in user who submitted that. I'm not sure that's a simple DB query though...

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#46

Post by SpoonMeiser »

thecoffman wrote: post #14
rmc wrote: post #10

I'm a Python/Django developer and don't really know much Ruby or Ruby on Rails. Does anyone have an outsiders/non-rubyist explaination of how this hack was carried out? Did he modify HTTP headers? POST parameters?

You would modify the hidden form values (or add parameters that didn't exist) depending on the situation. These new/modified parameters would appear in Rails' params hash which would then be passed to the update function which, by default, will update any fields you hand it.

This really doesn't sound like a problem with the update function, this just sounds like web applications 101 - don't trust user data.

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#47

Post by cbs »

why-el wrote: post #36
forza wrote: post #24

GitHub really needs to clearify if access to private repos was compromised, for how long and if such access would be traceable.

Between the time Homakov made his work public and the time it took them to fix it? I think it was close to an hour and on a Sunday. I doubt there are any other cases.

And we're confident that this guy is the only guy to ever exploit that weakness on github in the last 4 years?

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#48

Post by jarrett »

Homakov kind of gave the impression he had discovered a previously unknown vulnerability in Rails. This is not the case. Rather, he discovered an instance in which one very prominent Rails app (Github) failed to implement a standard Rails security practice.

For those not familiar with Rails, it boils down to this: You as the programmer need to use a security feature built into Rails called mass assignment security. If you fail to use this feature, you have a vulnerability. In other words, the default is insecure by design. The alternative would be to make Rails secure by default, but that would mean pretty much nothing would work until you explicitly granted access where necessary. I guess the core team figured "not working by default" was worse than "insecure by default."

Homakov obviously disagreed with this design decision. I can understand why, and I mostly feel the same way.

So Homakov posted an issue to the Rails repo Github (https://github.com/rails/rails/issues/5228) suggesting the default be changed. He made a good case and was initially polite. A few days passed, and nobody else had posted to his thread.

So, presumably to draw attention to this issue, he exploited the fact that Github had failed to use mass assignment protection. Specifically, he posted a comment with a far-future timestamp, which obviously should be impossible. (I think that's what he did, although Github seems to to have fixed the timestamp now.) He then said this should be proof enough that the Rails defaults need to be changed.

The problem with Homakov's argument, as pointed out in subsequent comments in the thread, is that Homakov's hack only demonstrated a mistake on Github's part, not a bug in Rails. It didn't prove anything about Rails that we didn't already know. The only thing surprising he demonstrated was that Github had left open a rather serious vulnerability.

TL;DR: Rails has some less-than-secure defaults which all Rails developers are expected to understand and deal with. Homakov found out that Github failed to do so in at least one instance, and he wanted to use that as proof the Rails defaults should be changed.

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#49

Post by ovi256 »

danso wrote: post #11

I hope those who ripped into Homakov initially are feeling more mellow towards him. They certainly were justified in thinking him being rash, but his action raised far more awareness about this issue than the typical proper channel. Rails devs (some, not all of them) had dismissed his complaint because "Every Rails developer hears about attr_accessible." Well, I'll be the first to say that I can't remember the last t…

On that checklist remark, I'd like to see and use more checklists. Hopefully some smarter person will write the "How to put a Rails app to prod" checklist, now that the need is obvious. Same for Django, or any other popular stack. It would be great if the communities created and maintained checklists summarizing best practices for different tasks.

Re: How Homakov hacked GitHub & the line of code that could have prevented it

#50

Post by ajross »

gcampbell wrote: post #34
o1iver wrote: post #25

"Homakov PUT an update to his own existing public key which included a new user_id. The user_id he used was that of a member of the Rails repository members." But wouldn't that mean, that the commit would display the username of the user with the `user_id' that he used?

Github displays the username/avatar for a commit based on that commit's Author field, not the user that pushed the changes to the repository.

It's more that they display both. Unlike git core, github actually tracks "push" events to branches (git doesn't care, it only sees commits) as distinct from the commits they contain.

So presumably it would have shown the rails developer pushing a change authored by Homakov.

Post reply on HN