-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Promote Hardcoded JWT credential query #12032
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
Java: Promote Hardcoded JWT credential query #12032
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
lib
for 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.java
file 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
QHelp previews: |
bf409a7
to
b259088
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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! 👍
Ok I have removed the unneeded file. |
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.
d1e2756
to
ed1aac1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DCA looks good! 👍
Query promotion for the experimental
java/hardcoded-jwt-key
by integrating it intojava/hardcoded-credential-api-call
.Promotion of #9036.