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

Example unsigned token that bypasses verification #364

Closed
brianlow opened this issue Jul 23, 2020 · 8 comments · Fixed by #365
Closed

Example unsigned token that bypasses verification #364

brianlow opened this issue Jul 23, 2020 · 8 comments · Fixed by #365

Comments

@brianlow
Copy link

brianlow commented Jul 23, 2020

you need to specify the algorithm in the options hash whenever you call JWT.decode to ensure that an attacker cannot bypass the algorithm verification step

Does anyone have an example of such a token?

I thought our system was exposed but I've been unable to write a failing unit test. For example:

malicious_token = JWT.encode({hi: 1}, nil, 'none')
# => "eyJhbGciOiJub25lIn0.eyJoaSI6MX0."

JWT.decode malicious_token, 'secret'
# => JWT::DecodeError: Not enough or too many segments

I cannot get JWT.decode(t, secret) to accept an unsigned token.

@danleyden
Copy link
Contributor

@brianlow eyJhbGciOiJub25lIn0.eyJpc3MiOiJqb2UiLA0KICJleHAiOjEzMDA4MTkzODAsDQogImh0dHA6Ly9leGFtcGxlLmNvbS9pc19yb290Ijp0cnVlfQ. is an example unsigned token (taken from https://tools.ietf.org/html/rfc7519#section-6.1)

For your decode call, you probably want to call something like JWT.decode(t, secret, true, { algorithms: %w(list of algorithms you are ok with)})

I believe that you've hit a bug with the encoder... In the case that the algorithm is none the encoder is returning a token that is not syntactically valid (it is missing the header declaring the algorithm). This bug was introduced by a refactor in the encoder in 43a08a4

@danleyden
Copy link
Contributor

Actually, I take it all back... there isn't a bug with the encoder... (I blame not enough coffee yet).

The "issue" here is that the decode call is trying to verify a signature on the token and a token with algorithm none has no signature to verify. The error message just isn't clear...

By default, JWT.decode sets the optional parameter verify to true, which is what you want in most cases. This will cause any token without a signature (e.g. with algorithm none) to fail decoding.

A "nicer" behaviour would be to raise an JWT::IncorrectAlgorithm error in this case (where verify is true - still check the claims - and the algorithm none is not specifically allowed)

@brianlow
Copy link
Author

That sound reasonable.

So it feels like using JWT.decode(token, password) is not vulnerable to attack by setting the token algorithm to none because it will raise an exception (though perhaps not an ideal one).

Is that fair?

@danleyden
Copy link
Contributor

Correct that it is not vulnerable to an attack whereby the attack uses the 'none' algorithm.

It may be vulnerable to other attacks. To reduce the attack surface, you should specify which algorithms you will allow the library to use when decoding. Not doing so may leave your application vulnerable to attacks such as:

  • providing a token signed using an algorithm that has weaknesses making it appear valid
  • using an asymmetric public key as a symmetric key to sign using a symmetric algorithm

If you are issuing the tokens, you should know what algorithm(s) you're using to sign the token. If you're not issuing the token, the issuer should be able to tell you what algorithms they use.

Hence advice to use: JWT.decode(token, password, true, { algorithms: %w(list of algorithms you are ok with)}) instead of just token and password.

@excpt excpt linked a pull request Jul 24, 2020 that will close this issue
@brianlow
Copy link
Author

brianlow commented Jul 24, 2020

@danleyden Thanks for the explanation! This really helps understand our risk. I'll pin the algorithm

@Nazzal1
Copy link

Nazzal1 commented Mar 29, 2024

سحب

@Gdish88
Copy link

Gdish88 commented Apr 29, 2024

Cash

@Gdish88
Copy link

Gdish88 commented Apr 29, 2024

سحب

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants