Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

FacebookOAuthToken should use facebook id instead of name - possibility to hijack account !! #6

Closed
snimavat opened this Issue Jul 29, 2013 · 12 comments

Comments

Projects
None yet
2 participants
Collaborator

snimavat commented Jul 29, 2013

Currently FacebookOAuthToken uses user.name as principal, so there is possibility of collision and even account can be hijacked. facebook user object's name property is full name of user and doesnt guarantee uniqueness. I can change my FB accounts name to 'Peter Ledbrook' and hijack your account ?

In case of facebook, OauthRealm uses user's full name to identify if he has an associated local account. FacebookOauthToken should use user's facebook id instead which can not change over time and guarantee uniqueness.

FacebookShiroOAuthService.createAuthToken needs to change.

See
https://developers.facebook.com/docs/reference/api/user/

Thanks

Collaborator

snimavat commented Jul 29, 2013

The same issue is present With google oauth as well.

Collaborator

snimavat commented Jul 29, 2013

Peter - I see that these issues are fixed in master branch but new version of plugin has not been released yet.
This is really critical, can you release a new version plz. 0.2 still doesnt have the fix.

Owner

pledbrook commented Jul 29, 2013

I don't see a fix for the Google OAuth though. I can release a 0.3-SNAPSHOT. The trouble is that I'm not actively using this at the moment. It was mainly done for the grails.org website which is no longer my responsibility.

If you'd like the authority to commit directly to the repo and publish the plugin yourself, let me know. I can help with questions, but not so much on the coding front.

Collaborator

snimavat commented Jul 29, 2013

I don't see a fix for the Google OAuth though

Google Oauth currently uses email, shouldn't that be fine ? its going to be unique and constant.

If you can release new version, that's fine, if you want to give me authority that's fine as well, I can release and do fixes if comes up.

Collaborator

snimavat commented Jul 30, 2013

There's another another security hole with linkAccount action in shiroOauthController.
If /$controller/$action mapping is enabled, I can proceed with oauth login and when it asks me to link existing account or create new one, instead of submitting form I can directly hit /shiroOauth/linkAccount?userId=2 and that will let me link my oauth account to any user. User id's might be guessable or I may find a user's id from profile links etc.

While linking account, I think it should first log user in using username/password token and then linkAccount action should use just currently logged in user's identity instead of excepting it in params.

Owner

pledbrook commented Jul 30, 2013

Ummm...you said the same issue is present with Google OAuth 😄 I'll assume it's fine.

I'll push out a snapshot release today for testing and at least give you commit access to this repo. I may be able to add publish rights for the plugin too, but need to verify.

Owner

pledbrook commented Jul 30, 2013

@snimavat Could you raise the linkAccount problem as a separate issue please?

Collaborator

snimavat commented Jul 30, 2013

Done

Owner

pledbrook commented Jul 30, 2013

What's your grails.org ID? snimavat? I'll request that you're given permission to publish the plugin.

@pledbrook pledbrook closed this Jul 31, 2013

Collaborator

snimavat commented Jul 31, 2013

@pledbrook yes, snimavat

Owner

pledbrook commented Aug 1, 2013

@snimavat You can now publish versions of the plugin to Grails Central as well as commit direct to this repository.

Collaborator

snimavat commented Aug 1, 2013

Yes, I noticed mail. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment