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

Fix prototype pollution vulnerability. #272

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Fix prototype pollution vulnerability. #272

merged 1 commit into from
Feb 21, 2022

Conversation

ryanpcmcquen
Copy link
Contributor

@ryanpcmcquen ryanpcmcquen commented Jan 28, 2022

Signed-off-by: Ryan McQuen RyanMcQuen@stockx.com

Description

Closes security vulnerability by adopting main lodash package, outlined here:
#271

References

Testing

  • This change adds test coverage for new/changed/fixed functionality No change in functionality.

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs No change in functionality.
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

Signed-off-by: Ryan McQuen <RyanMcQuen@stockx.com>
@ryanpcmcquen
Copy link
Contributor Author

Test output:

λ npm test

> express-jwt@6.1.0 test /Users/ryanmcquenstockx/code/express-jwt
> mocha --reporter spec



  failure tests
    ✓ should throw if options not sent
    ✓ should throw if algorithms is not sent
    ✓ should throw if algorithms is not an array
    ✓ should throw if no authorization header and credentials are required
    ✓ support unless skip
    ✓ should skip on CORS preflight
    ✓ should throw if authorization header is malformed
    ✓ should throw if authorization header is not Bearer
    ✓ should next if authorization header is not Bearer and credentialsRequired is false
    ✓ should throw if authorization header is not well-formatted jwt
    ✓ should throw if jwt is an invalid json
    ✓ should throw if authorization header is not valid jwt
    ✓ should throw if audience is not expected
    ✓ should throw if token is expired
    ✓ should throw if token issuer is wrong
    ✓ should use errors thrown from custom getToken function
    ✓ should throw error when signature is wrong
(node:72066) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
    ✓ should throw error if token is expired even with when credentials are not required
    ✓ should throw error if token is invalid even with when credentials are not required

  work tests
    ✓ should work if authorization header is valid jwt
    ✓ should work with nested properties
    ✓ should work if authorization header is valid with a buffer secret
    ✓ should set userProperty if option provided
    ✓ should set resultProperty if option provided
    ✓ should ignore userProperty if resultProperty option provided
    ✓ should work if no authorization header and credentials are not required
    ✓ should not work if no authorization header
    ✓ should produce a stack trace that includes the failure reason
    ✓ should work with a custom getToken function
    ✓ should work with a secretCallback function that accepts header argument

  multitenancy
    ✓ should retrieve secret using callback
    ✓ should throw if an error ocurred when retrieving the token
    ✓ should fail if token is revoked

  revoked jwts
    ✓ should throw if token is revoked
    ✓ should work if token is not revoked
    ✓ should throw if error occurs checking if token is revoked

  string tokens
    ✓ should work with a valid string token


  37 passing (33ms)

Copy link

@sirgoofiguskid sirgoofiguskid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new lodash version resolves known vulnerabilities. Checks pass.

@ryanpcmcquen
Copy link
Contributor Author

@mcastany can you take a look at this?

@sladg
Copy link

sladg commented Feb 9, 2022

Hey! Waiting for this change, can we make it happen? CC @mcastany

@ryanpcmcquen
Copy link
Contributor Author

Trying some more people from recent commits, seeing Snyk scans fail daily due to this:
@pipeline1987 @lbalmaceda

@pipeline1987
Copy link
Contributor

Trying some more people from recent commits, seeing Snyk scans fail daily due to this: @pipeline1987 @lbalmaceda

maybe @jfromaniello can you help us here? 👀

@ryanpcmcquen
Copy link
Contributor Author

@gkwang @aarongodin trying some more peeps since failing builds are no fun.

@gkwang
Copy link
Contributor

gkwang commented Feb 15, 2022

@ryanpcmcquen Let me get the right people to have this reviewed and merged.

@ryanpcmcquen
Copy link
Contributor Author

@gkwang thank you!

@mcastany mcastany merged commit 691fd6a into auth0:master Feb 21, 2022
@ryanpcmcquen ryanpcmcquen deleted the prototype-pollution-vulnerability-fix branch February 21, 2022 15:09
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 this pull request may close these issues.

None yet

7 participants