Add EC to secure algorithm whitelist for Java CWE-327 query#21594
Add EC to secure algorithm whitelist for Java CWE-327 query#21594owen-mc merged 4 commits intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Java CWE-327 “potentially weak cryptographic algorithm” query support code to treat common Elliptic Curve (EC) algorithm names as secure, addressing reported false positives.
Changes:
- Extend the secure-algorithm whitelist in
Encryption.qllto include EC/ECDH/ECDSA and related modern curve families (EdDSA/Ed25519/Ed448, XDH/X25519/X448). - Add query-test coverage examples intended to ensure these algorithms are no longer flagged.
- Add a change note documenting the reduction in false positives.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| java/ql/lib/semmle/code/java/security/Encryption.qll | Expands the secure algorithm name whitelist to include EC-family algorithm identifiers. |
| java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java | Adds “GOOD” examples for EC-family algorithm strings to prevent regressions in CWE-327 behavior. |
| java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md | Documents the analysis-result change (fewer false positives). |
java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java
Outdated
Show resolved
Hide resolved
java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java
Outdated
Show resolved
Hide resolved
owen-mc
left a comment
There was a problem hiding this comment.
Thanks for this contribution. It looks good, and should be mergeable with a few small changes. Please add signature.getInstance as a sink, as I explained in another comment.
In another forum, a user complained about FPs with this query, and gave the below examples. Is it possible you could add them at the same time?
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import javax.crypto.spec.SecretKeySpec;
import javax.crypto.SecretKeyFactory;
import java.security.KeyPairGenerator;
public class C {
public static void main(String[] args) {
try {
MessageDigest msgDigester1 = MessageDigest.getInstance("MD5"); //OK
MessageDigest msgDigester2 = MessageDigest.getInstance("SHA-1"); //OK
MessageDigest msgDigester3 = MessageDigest.getInstance("SHA-256"); //OK
new SecretKeySpec(null, "HMACSHA1"); // FP
new SecretKeySpec(null, "HMACSHA256"); // FP
new SecretKeySpec(null, "HMACSHA384"); // FP
new SecretKeySpec(null, "SHA384withECDSA"); // FP
SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); // FP
KeyPairGenerator.getInstance("EC"); // FP
} catch (NoSuchAlgorithmException nsae) {
System.out.println("NoSuchAlgorithmException caught when digest token");
}
}
}
java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md
Outdated
Show resolved
Hide resolved
|
no problem. I will handle all these issues you mentioned ASAP. Will let u know when ready. :) |
…ist, fix test APIs - Model Signature.getInstance() as CryptoAlgoSpec sink (previously only Signature constructor was modeled) - Add HMAC-based algorithms (HMACSHA1/256/384/512, HmacSHA1/256/384/512) and PBKDF2 to the secure algorithm whitelist - Fix XDH/X25519/X448 tests to use KeyAgreement.getInstance() instead of KeyPairGenerator.getInstance() to match their key agreement semantics - Add test cases for SHA384withECDSA, HMACSHA*, and PBKDF2WithHmacSHA1 from user-reported false positives - Update change note to document all additions
80880b4 to
da4a223
Compare
|
Added all examples. Also added HMAC/PBKDF2 to whitelist and updated change note. |
fix #21593