mirror of
https://github.com/github/codeql.git
synced 2026-04-26 09:15:12 +02:00
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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`. */
|
||||
|
||||
Reference in New Issue
Block a user