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

Inconsistent use of symbol and string keys in args (exp and alrogithm). #331

Closed
hesalx opened this issue Sep 10, 2019 · 1 comment · Fixed by #359
Closed

Inconsistent use of symbol and string keys in args (exp and alrogithm). #331

hesalx opened this issue Sep 10, 2019 · 1 comment · Fixed by #359

Comments

@hesalx
Copy link

hesalx commented Sep 10, 2019

  1. [Applies to <= 2.1.0, fixed in >= 2.2.0] While it is possible to use symbolized claim names everywhere, the exp claim is only validated if passed as string key to encode.
> JWT.encode({ 'exp' => 'asd' }, 'key')
JWT::InvalidPayload: exp claim must be an integer

> JWT.encode({ exp: 'asd' }, 'key')
=> "eyJhbGciOiJIUzI1NiJ9.eyJleHAiOiJhc2QifQ.vMAZ6k88kjdSq9UW_raFMNlhBGz2L01a_4o_ZI1SCgU"

Which may lead to unexpected results:

> token = JWT.encode({ exp: "#{Time.now + 2629746}" }, 'key')
=> "eyJhbGciOiJIUzI1NiJ9.eyJleHAiOiJhc2QifQ.vMAZ6k88kjdSq9UW_raFMNlhBGz2L01a_4o_ZI1SCgU"
> JWT.decode(token, 'key')
JWT::ExpiredSignature: Signature has expired
  1. While it is possible to use string keys everywhere, algorithm/algorithms is requred to be a symbol, while being in the same options hash where string keys work perfectly fine.
> token = JWT.encode({ 'aud' => 'foo'}, 'key')
=> "eyJhbGciOiJIUzI1NiJ9.e30.FOOB1UqKAhjxqs3lV7BidJ12zFAsIGq1erQfNyaFf80"
> JWT.decode(token, 'key', true, 'aud' => 'foo', 'verify_aud' => true, 'algorithm' => 'HS384')
=> [{"aud"=>"foo"}, {"alg"=>"HS256"}] # not the allowed algorithm!

Gladly, none is not an allowed algorithm in the payload by default, but if this logic is changed in a wrongfull manner (see #323 as a possible cause of such changes, for example) it is going to create a security whole immediately regarding misued string 'algorithm' key.

Anyway, even without the known "alg none" issue, a possibility to unknowingly break algorith validation is concerning.

@hesalx
Copy link
Author

hesalx commented Oct 2, 2019

Didn't notice we've been using sighly older verison. The first issue (exp validation) is actually fixed (can't be reproduced) in 2.2.1.

Though the alg issue is still valid for 2.2.1.

@excpt excpt self-assigned this Jul 7, 2020
@excpt excpt closed this as completed in #359 Jul 8, 2020
excpt added a commit that referenced this issue Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants