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

31–40 of 91 posts

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

#31

Post by adriand »

phamilton wrote: post #21
adriand wrote: post #15

I agree that the default for Rails should be more secure, but this was a really basic mistake by the GitHub team. Yes, mistakes do happen, but there's a very simple way to avoid the exploit that is postulated in this article. The 'schematic' of what the public key update looks like from the original post: class PublicKeyController The correct way to code this is as follows: class PublicKeyController This has two upda…

That wouldn't fix it. I own the key. I set the owner of the key to you. You now own the key that I have on my machine. I commit to your repo. I'm sure the github team has authorization at a model level, preventing access to other user's resources. This wasn't an instance of accessing another user's resources via rails, it was assigning your own resources to another user, then abusing that fact via git.

Actually, that's true. I stand corrected. I think I was distracted by the rather obvious issue in the example code that the OP posted, which did not include this basic protection.

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

#32

Post by phamilton »

adriand wrote: post #28
jgrahamc wrote: post #19

Earlier quoted context omitted.

Doesn't the line: @current_key.update_attributes(params[:key]) still mean that an attacker can pass in arbitrary fields to be updated in the public key table? If that's the case then doesn't it open room for an attacker to change a field that is assumed not accessible from the outside? For example, would it be possible to change an _id field that references another table?

Yes, if there is anything else in the public key table that you don't want to be updated via form fields, then you should be protecting yourself against it. However, every developer - or at minimum, every experienced developer - should know the basic maxim "never trust the data from your users". But this method does protect resources from being arbitrarily assigned to other users. In any controller where the user sho…

I tend to use the declarative gem:

PublicKey.with_permissions_to(:write)

and then define the scoping rules in the declarative auth file:

authorization do role :user do has_permission_on :public_keys do to [:write, :read] # user refers to the current_user when evaluating if_attribute :user_id => is {user.id} end end end

This is a bit more DRY, because you are abstract out the conditions of access. This is especially useful in situations where you have readonly access or other types of acl.

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

#33

Post by danso »

phamilton wrote: post #26
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?

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…

Really? How is this exotic? Simple inspection of the rails form will reveal attribute and model names. And as Homarov proves, you don't need to even leave the friendly interface of the inspector to POST what you want.

The only reason why a cracker hasn't tried this is because it seems too simple to work.

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

#34

Post by gcampbell »

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.

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

#35

Post by judofyr »

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.

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

#36

Post by why-el »

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.

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

#37

Post by unclegene »

Am I the only one who does not understand what is this about? Oh, no, looks like rails team does neither. Stupid code can be written in any framework/language. How much experience does one need to understand a simple rule - _never_ use user input directly. If you have an urge to trust your users - I'd suggest better way: `params[:command]`

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

#38

Post by Estragon »

phamilton wrote: post #26
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?

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

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

#39

Post by davidblondeau »

gphil wrote: post #8

I posted a link to the relevant part of it ( http://news.ycombinator.com/item?id=3665429 ) on another thread regarding this exploit already, but the official Rails Security Guide covers this and other common security pitfalls really well. It is worth reading over thoughtfully if you are building a Rails app: http://guides.rubyonrails.org/security.html

Right, the Rails security guide and the Netscape secure coding guide (https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines) are highly recommended reading for every developer at my company. They are very well written and cover a lot of ground when developing web applications on RoR.
Post reply on HN