Java: Promote Hardcoded JWT credential query #12032
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
owen-mc
changed the title
jcogs33
reviewed
Feb 15, 2023
Contributor
There was a problem hiding this comment.
Hey Ed, I’ve added a few comments/questions below, but I’ll leave a full review and approval to someone else. 🙂
- This will probably need a change note under
libfor the new sinks. (FYI, it’s standard to mention the original author of the experimental query in the change note when doing a promotion, see this discussion) - Where is the
java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.javafile being used? Was this meant to be added as an example inHardcodedCredentialsApiCall.qhelp? - Would it make sense to include test cases for all of the new sinks? e.g. HMAC384 and HMAC512 as well?
java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java
Outdated
Show resolved
Hide resolved
Contributor
|
QHelp previews: |
egregius313
force-pushed
the
egregius313/promote-hardcoded-jwt-credential
branch
from
bf409a7 to
b259088
Compare
atorralba
reviewed
Feb 20, 2023
Contributor
atorralba
left a comment
atorralba
left a comment
There was a problem hiding this comment.
The HardcodedJwtKey.java file under Security/CWE/CWE-798 isn't doing much unless you reference it in the QHelp. So I'd recommend doing that, or just removing it.
But otherwise this looks good to me! 👍
Contributor
Author
Ok I have removed the unneeded file. |
atorralba
added
ready-for-doc-review
The HMAC* constructors of the com.auth0.jwt.algorithm.Algorithm class take a secret as a parameter. Therefore, the arguments should be added to be checked for hardcoded credentials.
atorralba
force-pushed
the
egregius313/promote-hardcoded-jwt-credential
branch
from
d1e2756 to
ed1aac1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Query promotion for the experimental
java/hardcoded-jwt-keyby integrating it intojava/hardcoded-credential-api-call.Promotion of #9036.