Skip to content

possible to hijack account : security hole with linkAccount action #7

Closed
snimavat opened this Issue Jul 30, 2013 · 7 comments

3 participants

@snimavat
Collaborator

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.

@pledbrook
Owner

Yeah. A bit awkward. I'll take a quick look.

@pledbrook
Owner

The linkAccount action is supposed to require an authenticated user (via username/password combination). I think this may be highlighting the limits of my Shiro knowledge. I guess the OAuth authentication is counting as 'authenticated', although I don't think it should be.

Unfortunately, it's hard to test this stuff.

@pledbrook
Owner

OK, so the action doesn't require an authenticated user. The trouble is we need to handle both new users as well as those with existing accounts.

You're right though, we shouldn't accept the link account ID via a parameter. Or the password should be passed in with the user ID.

@snimavat
Collaborator

Right, If we can have it like, when creating new ShiroOauthIdentity (shiroOauthController.linkAccount), user must be logged in and we use just SecurityUtils.subject.principal for linking. oauthConfig.linkAccountUrl endpoint will actually have to log user in using existing username/password or create new user based on submitted details and log in new user and thn forward request to ShiroOAuthcontroller.linkAccount. linkAccount will create new ShiroOauthIdentity based on SecurityUtils.subject.principal if one doesn't already exist for the given provider. This way linkAccount action will never let user associate his oauth to any other account other thn self.

@pledbrook pledbrook added a commit that referenced this issue Jul 30, 2013
@pledbrook Fix security issue in linkAccount.
As described in issue #7, it was possible to link an OAuth account to *any*
internal account if the ID of the user record was known (or guessed). The
`linkAccount` action now requires either a username/password combination or
an authenticated user with a principal of type `long` in order to complete
the linking.
2e0bd78
@pledbrook
Owner

I have pushed a proposed change to the issue7 branch and also published a 0.3-SNAPSHOT. Let me know if it works for you. The key change is that now the linkAccount action won't accept a userId parameter. Instead, you either provide a username and password or pre-authenticate the user with an account that has a principal of type long (such as that provided by the default DB realm).

@snimavat
Collaborator

Works Great !

Thanks

@pledbrook pledbrook closed this Jul 31, 2013
@concealed

Hello, I am still very new to shiro, but I am having issues with this linkAccount action. The problem is that I am not getting a userId from the line subject.principals.oneByType(Long). I am using the grails shiro plugin and it created a ShiroDbRealm for me. Is the the default DB realm you are mentioning, pledbrook? Because it uses the username as the principal instead of userid. Any help will be highly appreciated. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.