Check JWT vulnerability

[desmondhume]

Can somebody tell me if this gem is affected by this vulnerability in some way?

Thanks

[danleyden]

It may be depending on how you are using it.

If you disable verification when calling decode, you are vulnerable.

Assuming you do not disable verification, you may still be vulnerable. It is possible to specify the algorithm in the options passed to decode. If you enforce the algorithm, then your application will not be exposed to that vulnerability.

If you do not enforce the algorithm when decoding (or must allow more than one algorithm), depending on the implementation of your keyfinder algorithm passed in, your application may be vulnerable.

[thedeeno]

@danleyden I think the reliance on the alg header should be dropped and decode should accept a parameter for the algorithm like the author of the op's article suggests. What do you think?

I can patch this. I would replace decode's second parameter 'verify' with a string that specifies the algorithm. If a consumer wishes to skip verification they can use the algorithm "NONE". I can assume false means "NONE" to avoid a break (though if consumers specified true for verify I'd have to throw). I think the vulnerability should be fixed at the library level though, not application.

[danleyden]

@thedeeno The alg header is really important and useful in some use cases and its use should not be dropped - it is an important (required) part of the spec.

Take for example the case where multiple algorithms are acceptable to the service receiving the token. In order to ensure that the token is valid, the library needs to know specifically which algorithm was used to generate the signature (or encrypt the token for that matter).

I do not believe that altering the semantics of the second parameter (verify) is a good idea. That would break backwards compatibility (including for myself) as that parameter controls all verification, not just the algorithm.

The functionality to allow the application to specify the algorithm is already in place via the options parameter. The consumer can (in most cases should) provide algorithm when calling decode using something along the lines of: JWT.decode(token, key, true, algorithm: 'RS512').

Given that this is already in place, it may be better to improve the documentation here to point consumers at best practice ways of using the library rather than break backwards compatibility. It may be a good idea to have this better practice more enforced in version 2 (which I know there has been some work on).

[thedeeno]

@danleyden I think this library should be secure by default. It makes me nervous that consumers need to use optional parameters to protect against vulnerabilities.

I think breaking backwards compatibility to close a security vulnerability is worth it. If this requires a major version bump, then so be it.

Most people will use the simplest API available when starting to use this library; making beginners the most vulnerable.

I suspect the spec is going to change to reflect Tim McLean's recommendation. Giving an attacker control of the verification algorithm is likely a mistake.

Just my 2cents. Thanks for your hard work on this library!

[thedeeno]

@danleyden Hmm, it appears 1.4.1 doesn't support the :algorithm option. Master does. Perhaps a new release should be cut so users can work around this vulnerability without being forced to track head?

[thedeeno]

We can't track the head from Gemfile easily either, since there's no .gemspec in repo.

Sorry for the email spam.