From da4a2238bc706382a41d402e5a907d9d95d6424a Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 28 Mar 2026 16:51:13 +0800 Subject: [PATCH] Address PR review: add Signature.getInstance sink, HMAC/PBKDF2 whitelist, 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 --- .../2026-03-27-add-ec-to-secure-algorithms.md | 3 ++- .../semmle/code/java/security/Encryption.qll | 10 +++++++-- .../security/CWE-327/semmle/tests/Test.java | 21 +++++++++++++------ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md b/java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md index 2c46d38ebfe..1e323fafd35 100644 --- a/java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md +++ b/java/ql/lib/change-notes/2026-03-27-add-ec-to-secure-algorithms.md @@ -1,4 +1,5 @@ --- category: minorAnalysis --- -* The `java/potentially-weak-cryptographic-algorithm` query no longer flags Elliptic Curve algorithms (`EC`, `ECDSA`, `ECDH`, `EdDSA`, `Ed25519`, `Ed448`, `XDH`, `X25519`, `X448`) as potentially insecure. These are modern, secure algorithms recommended by NIST SP 800-57 and other standards bodies. Previously, these algorithms were not included in the secure algorithm whitelist, causing false positives when using standard Java cryptographic APIs such as `KeyPairGenerator.getInstance("EC")`. +* The `java/potentially-weak-cryptographic-algorithm` query no longer flags Elliptic Curve algorithms (`EC`, `ECDSA`, `ECDH`, `EdDSA`, `Ed25519`, `Ed448`, `XDH`, `X25519`, `X448`), HMAC-based algorithms (`HMACSHA1`, `HMACSHA256`, `HMACSHA384`, `HMACSHA512`), or PBKDF2 key derivation as potentially insecure. These are modern, secure algorithms recommended by NIST and other standards bodies. Previously, these algorithms were not included in the secure algorithm whitelist, causing false positives when using standard Java cryptographic APIs such as `KeyPairGenerator.getInstance("EC")` or `new SecretKeySpec(key, "HMACSHA256")`. +* The `Signature.getInstance(...)` method is now modeled as a `CryptoAlgoSpec` sink, alongside the existing `Signature` constructor sink. This ensures that algorithm strings passed to `Signature.getInstance(...)` are also checked by the query. diff --git a/java/ql/lib/semmle/code/java/security/Encryption.qll b/java/ql/lib/semmle/code/java/security/Encryption.qll index afbace5bf45..6af8d29cc4a 100644 --- a/java/ql/lib/semmle/code/java/security/Encryption.qll +++ b/java/ql/lib/semmle/code/java/security/Encryption.qll @@ -263,7 +263,9 @@ string getASecureAlgorithmName() { // Elliptic Curve algorithms: EC (key generation), ECDSA (signatures), ECDH (key agreement), // EdDSA/Ed25519/Ed448 (Edwards-curve signatures), XDH/X25519/X448 (key agreement). // These are modern, secure algorithms recommended by NIST and other standards bodies. - "EC", "ECDSA", "ECDH", "EdDSA", "Ed25519", "Ed448", "XDH", "X25519", "X448" + "EC", "ECDSA", "ECDH", "EdDSA", "Ed25519", "Ed448", "XDH", "X25519", "X448", + // HMAC-based algorithms and key derivation functions. + "HMACSHA(1|256|384|512)", "HmacSHA(1|256|384|512)", "PBKDF2" ] } @@ -370,9 +372,13 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec { exists(Constructor c | c.getAReference() = this | c.getDeclaringType().hasQualifiedName("java.security", "Signature") ) + or + exists(Method m | m.getAReference() = this | + m.hasQualifiedName("java.security", "Signature", "getInstance") + ) } - override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) } + override Expr getAlgoSpec() { result = this.(Call).getArgument(0) } } /** A call to the `getInstance` method declared in `java.security.KeyPairGenerator`. */ diff --git a/java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java b/java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java index 41ce1e69784..23aff65161c 100644 --- a/java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java +++ b/java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java @@ -52,7 +52,7 @@ class Test { // GOOD: EC is a secure algorithm for key pair generation keyPairGenerator = KeyPairGenerator.getInstance("EC"); - // GOOD: ECDSA is a secure algorithm for digital signatures + // GOOD: ECDSA is a secure signature algorithm Signature ecdsaSig = Signature.getInstance("ECDSA"); // GOOD: ECDH is a secure algorithm for key agreement @@ -61,24 +61,33 @@ class Test { // GOOD: EdDSA is a secure algorithm (Edwards-curve Digital Signature Algorithm) keyPairGenerator = KeyPairGenerator.getInstance("EdDSA"); - // GOOD: Ed25519 is a secure algorithm + // GOOD: Ed25519 is a secure algorithm for key pair generation keyPairGenerator = KeyPairGenerator.getInstance("Ed25519"); - // GOOD: Ed448 is a secure algorithm + // GOOD: Ed448 is a secure algorithm for key pair generation keyPairGenerator = KeyPairGenerator.getInstance("Ed448"); // GOOD: XDH is a secure algorithm for key agreement - keyPairGenerator = KeyPairGenerator.getInstance("XDH"); + KeyAgreement xdhKa = KeyAgreement.getInstance("XDH"); // GOOD: X25519 is a secure algorithm for key agreement - keyPairGenerator = KeyPairGenerator.getInstance("X25519"); + KeyAgreement x25519Ka = KeyAgreement.getInstance("X25519"); // GOOD: X448 is a secure algorithm for key agreement - keyPairGenerator = KeyPairGenerator.getInstance("X448"); + KeyAgreement x448Ka = KeyAgreement.getInstance("X448"); // GOOD: SHA256withECDSA is a secure signature algorithm Signature sha256Ecdsa = Signature.getInstance("SHA256withECDSA"); + // GOOD: HMAC-based SecretKeySpec should not be flagged + new SecretKeySpec(null, "HMACSHA1"); + new SecretKeySpec(null, "HMACSHA256"); + new SecretKeySpec(null, "HMACSHA384"); + new SecretKeySpec(null, "SHA384withECDSA"); + + // GOOD: PBKDF2 key derivation is a secure algorithm + SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + } catch (Exception e) { // fail }