Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[security] Signature verified after expiration/sub/iss checks #153

Closed
koenbollen opened this issue Jun 8, 2016 · 3 comments
Closed

[security] Signature verified after expiration/sub/iss checks #153

koenbollen opened this issue Jun 8, 2016 · 3 comments

Comments

@koenbollen
Copy link

Given a feature were you login using an JWT and get redirected to an URL that is inside the claims of the JWT, when an JWT is expired you are still redirected without logging in.

Because the expiration is checked before the validity of the signature an JWT::ExpiredSignature is raise before the signature is checked. This allows attackers to create a malicious token, with a bogus signature, that is expired but includes a redirect claim making an open redirect.

You could fix it by doing an decode with verify_expiration: false first, before decoding it again with all verifies on true.

Off course, you shouldn't add a feature on expired tokens but I would expect that the validity is checked before the checking the claims. Furthermore, this is just on example where it could go wrong.

It's about this code: https://git.io/votyr
I propose switching it around, verifying the validity of the signature first, and then the claims.

Thanks for the great Gem!

Cheers,
—Koen

@excpt
Copy link
Member

excpt commented Aug 22, 2016

@koenbollen can you please test your case with the #160 PR?

@excpt excpt added the WIP label Aug 22, 2016
@koenbollen
Copy link
Author

@excpt Great work! It has fixed the issue, I've used that branch and removed my extra check and got the expected result.
Thanks.

@excpt excpt removed the WIP label Aug 23, 2016
@excpt
Copy link
Member

excpt commented Aug 23, 2016

Thanks for the fast response. 👍

@excpt excpt closed this as completed Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants