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

Algorithm specification vulnerability for versions pre-2.0 running on JRuby #279

Open
revodoge opened this issue Sep 2, 2018 · 3 comments

Comments

@revodoge
Copy link
Contributor

revodoge commented Sep 2, 2018

Algorithm became a required param for verifying signature in #184 to address https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

The PR mentioned:

This doesn't seem to be exploitable right now because the current implementation
of OpenSSL::HMAC.digest expects a string as the key, so if rsa_public is an
OpenSSL::PKey::RSA object, JWT.decode will raise an error. But it would be
better not to depend on this OpenSSL::HMAC.digest behavior

Although the behavior mentioned holds for MRI, JRuby behaves differently and is vulnerable (i.e. you get a successful verification without any errors raised).

Out of caution, I'd recommend marking the old versions as vulnerable with a note clarifying that only JRuby is affected here: https://github.com/rubysec/ruby-advisory-db

That way people can get notified by tooling such as bundler audit of the potential need to upgrade

@excpt excpt self-assigned this Sep 2, 2018
@excpt excpt added this to the Version 2.2.0 milestone Sep 2, 2018
@excpt
Copy link
Member

excpt commented Sep 7, 2018

Hi @revodoge,

thank you very much for this report.

I am going to fill in the required form to get this issue into the database.

@excpt excpt removed this from the Version 2.2.0 milestone Jan 21, 2019
@victorhazbun
Copy link

@revodoge Does this affects version 2.2.1 and above? if not, why not? cc @excpt

@revodoge
Copy link
Contributor Author

@victorhazbun this was fixed in 2.0 by requiring algorithm to be passed in as a parameter and from a quick glance at the current code it looks like you still need algorithms passed in so this should be fine

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

No branches or pull requests

3 participants