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

[JS] - Enhancement to add jose decodeJWT to js/jwt-missing-verification #147

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

@felickz felickz changed the title Enhancement to add jose decodeJWT to MissingJWTKeyVerification.ql Enhancement to add jose decodeJWT to js/jwt-missing-verification May 19, 2023
@felickz felickz marked this pull request as ready for review May 19, 2023 18:09
@felickz felickz requested a review from GeekMasher as a code owner May 19, 2023 18:09
and node = call.getArgument(1)
and msg = "This argument disables the integrity enforcement of the token verification.")
or
(call = DataFlow::moduleMember("jose", "decodeJwt").getACall()
Copy link
Contributor Author

@felickz felickz May 19, 2023

Choose a reason for hiding this comment

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

Calling jose.decodeJwt method alone is not a fool proof detection as there is potential that the token is validated in another scenario ( jose.jwtVerify ). On that note should we consider dropping this into codeql repo or keep in this field pack?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep in Field for now. Have you checked this against MRVA to see what % FPs this leads to? If it's significant it can't be hard to add "and not jose.jwtVerify used", right?

@felickz felickz changed the title Enhancement to add jose decodeJWT to js/jwt-missing-verification [CSharp] - Enhancement to add jose decodeJWT to js/jwt-missing-verification May 19, 2023
@GeekMasher GeekMasher changed the title [CSharp] - Enhancement to add jose decodeJWT to js/jwt-missing-verification [JS] - Enhancement to add jose decodeJWT to js/jwt-missing-verification May 25, 2023
@pwntester
Copy link

This repo has been merged with the Security Lab one into the new community-codeql-packs repo which we plan to make public and promote soon. If you would like this PR to be applied to the new repo, please open a new PR there so it can get merged in the new QLPacks.

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

Successfully merging this pull request may close these issues.

3 participants