Skip to content

Commit 682545f

Browse files
sam-robsonsam-robson
authored
Merge pull request #23027 from github/sam-robson/java-weak-crypto-case-sensitivity
Java: Match mixed-case weak cryptographic algorithm strings
2 parents 090f0d8 + fbcefd3 commit 682545f

8 files changed

Lines changed: 37 additions & 11 deletions

File tree

‎java/ql/lib/change-notes/2026-11-05-weak-crypto-case-sensitivity.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `java/weak-cryptographic-algorithm`, `java/potentially-weak-cryptographic-algorithm`, and `java/rsa-without-oaep` queries now match cryptographic algorithm names and modes case-insensitively (for example, `AES/CBC/PKCS5PADDING` and `AES/cbc/Pkcs5Padding`).

‎java/ql/lib/semmle/code/java/security/BrokenCryptoAlgorithmQuery.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class BrokenAlgoLiteral extends ShortStringLiteral {
1717
BrokenAlgoLiteral() {
1818
this.getValue().regexpMatch(getInsecureAlgorithmRegex()) and
1919
// Exclude RSA/ECB/.* ciphers.
20-
not this.getValue().regexpMatch("RSA/ECB.*") and
20+
not this.getValue().regexpMatch("(?i)RSA/ECB.*") and
2121
// Exclude German and French sentences.
2222
not this.getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
2323
}

‎java/ql/lib/semmle/code/java/security/Encryption.qll‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,15 +189,18 @@ bindingset[algorithmString]
189189
private string algorithmRegex(string algorithmString) {
190190
// Algorithms usually appear in names surrounded by characters that are not
191191
// alphabetical characters in the same case. This handles the upper and lower
192-
// case cases.
192+
// case cases, plus mixed-case algorithm strings bounded by non-alphabetic
193+
// characters (such as `AES/cbc/Pkcs5Padding`).
193194
result =
194195
"((^|.*[^A-Z])(" + algorithmString + ")([^A-Z].*|$))" +
195196
// or...
196197
"|" +
197198
// For lowercase, we want to be careful to avoid being confused by camelCase
198199
// hence we require two preceding uppercase letters to be sure of a case switch,
199200
// or a preceding non-alphabetic character
200-
"((^|.*[A-Z]{2}|.*[^a-zA-Z])(" + algorithmString.toLowerCase() + ")([^a-z].*|$))"
201+
"((^|.*[A-Z]{2}|.*[^a-zA-Z])(" + algorithmString.toLowerCase() + ")([^a-z].*|$))" +
202+
// or...
203+
"|" + "((^|.*[^a-zA-Z])(?i)(" + algorithmString + ")([^a-zA-Z].*|$))"
201204
}
202205

203206
/**
@@ -258,7 +261,7 @@ string getInsecureAlgorithmReason(string input) {
258261
string getASecureAlgorithmName() {
259262
result =
260263
[
261-
"RSA", "SHA-?(256|384|512)", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))",
264+
"RSA", "SHA-?(256|384|512)", "CCM", "GCM", "AES(?![^a-zA-Z](?i)(ECB|CBC/PKCS[57]Padding))",
262265
"Blowfish", "ECIES", "SHA3-(256|384|512)",
263266
// Elliptic Curve algorithms: EC (key generation), ECDSA (signatures), ECDH (key agreement),
264267
// EdDSA/Ed25519/Ed448 (Edwards-curve signatures), XDH/X25519/X448 (key agreement).

‎java/ql/lib/semmle/code/java/security/RsaWithoutOaepQuery.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import semmle.code.java.dataflow.DataFlow
1010
module RsaWithoutOaepConfig implements DataFlow::ConfigSig {
1111
predicate isSource(DataFlow::Node src) {
1212
exists(CompileTimeConstantExpr specExpr, string spec |
13-
specExpr.getStringValue() = spec and
13+
specExpr.getStringValue().toUpperCase() = spec and
1414
specExpr = src.asExpr() and
1515
spec.matches("RSA/%") and
1616
not spec.matches("%OAEP%")

‎java/ql/test/query-tests/security/CWE-327/semmle/tests/BrokenCryptoAlgorithm.expected‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
11
#select
22
| Test.java:19:20:19:50 | getInstance(...) | Test.java:19:45:19:49 | "DES" | Test.java:19:45:19:49 | "DES" | Cryptographic algorithm $@ is insecure. It has a short key length of 56 bits, making it vulnerable to brute-force attacks. Consider using AES instead. | Test.java:19:45:19:49 | "DES" | DES |
33
| Test.java:42:14:42:38 | getInstance(...) | Test.java:42:33:42:37 | "RC2" | Test.java:42:33:42:37 | "RC2" | Cryptographic algorithm $@ is insecure. It is vulnerable to related-key attacks. Consider using AES instead. | Test.java:42:33:42:37 | "RC2" | RC2 |
4+
| Test.java:46:14:46:55 | getInstance(...) | Test.java:46:33:46:54 | "AES/CBC/PKCS5Padding" | Test.java:46:33:46:54 | "AES/CBC/PKCS5Padding" | Cryptographic algorithm $@ is insecure. CBC mode with PKCS#5 or PKCS#7 padding is vulnerable to padding oracle attacks. Consider using GCM instead. | Test.java:46:33:46:54 | "AES/CBC/PKCS5Padding" | AES/CBC/PKCS5Padding |
5+
| Test.java:47:14:47:55 | getInstance(...) | Test.java:47:33:47:54 | "AES/CBC/PKCS5PADDING" | Test.java:47:33:47:54 | "AES/CBC/PKCS5PADDING" | Cryptographic algorithm $@ is insecure. CBC mode with PKCS#5 or PKCS#7 padding is vulnerable to padding oracle attacks. Consider using GCM instead. | Test.java:47:33:47:54 | "AES/CBC/PKCS5PADDING" | AES/CBC/PKCS5PADDING |
6+
| Test.java:48:14:48:55 | getInstance(...) | Test.java:48:33:48:54 | "aes/cbc/pkcs5padding" | Test.java:48:33:48:54 | "aes/cbc/pkcs5padding" | Cryptographic algorithm $@ is insecure. CBC mode with PKCS#5 or PKCS#7 padding is vulnerable to padding oracle attacks. Consider using GCM instead. | Test.java:48:33:48:54 | "aes/cbc/pkcs5padding" | aes/cbc/pkcs5padding |
7+
| Test.java:49:14:49:55 | getInstance(...) | Test.java:49:33:49:54 | "AES/cbc/Pkcs5Padding" | Test.java:49:33:49:54 | "AES/cbc/Pkcs5Padding" | Cryptographic algorithm $@ is insecure. CBC mode with PKCS#5 or PKCS#7 padding is vulnerable to padding oracle attacks. Consider using GCM instead. | Test.java:49:33:49:54 | "AES/cbc/Pkcs5Padding" | AES/cbc/Pkcs5Padding |
48
edges
59
nodes
610
| Test.java:19:45:19:49 | "DES" | semmle.label | "DES" |
711
| Test.java:42:33:42:37 | "RC2" | semmle.label | "RC2" |
12+
| Test.java:46:33:46:54 | "AES/CBC/PKCS5Padding" | semmle.label | "AES/CBC/PKCS5Padding" |
13+
| Test.java:47:33:47:54 | "AES/CBC/PKCS5PADDING" | semmle.label | "AES/CBC/PKCS5PADDING" |
14+
| Test.java:48:33:48:54 | "aes/cbc/pkcs5padding" | semmle.label | "aes/cbc/pkcs5padding" |
15+
| Test.java:49:33:49:54 | "AES/cbc/Pkcs5Padding" | semmle.label | "AES/cbc/Pkcs5Padding" |
816
subpaths

‎java/ql/test/query-tests/security/CWE-327/semmle/tests/Test.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ public void test() {
4141
// BAD: RC2 is a weak algorithm
4242
cipher = Cipher.getInstance("RC2"); // $ Alert[java/weak-cryptographic-algorithm]
4343
}
44+
{
45+
// BAD: AES/CBC/PKCS5Padding is vulnerable to padding oracle attacks, regardless of case
46+
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); // $ Alert[java/weak-cryptographic-algorithm]
47+
cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING"); // $ Alert[java/weak-cryptographic-algorithm]
48+
cipher = Cipher.getInstance("aes/cbc/pkcs5padding"); // $ Alert[java/weak-cryptographic-algorithm]
49+
cipher = Cipher.getInstance("AES/cbc/Pkcs5Padding"); // $ Alert[java/weak-cryptographic-algorithm]
50+
}
4451
// GOOD: ECIES is a strong algorithm
4552
cipher = Cipher.getInstance("ECIES");
4653
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec);

‎java/ql/test/query-tests/security/CWE-780/RsaWithoutOaepTest.expected‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
#select
22
| RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | This specification is used to $@ without OAEP padding. | RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | initialize an RSA cipher |
3-
| RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:11:35:11:38 | spec | This specification is used to $@ without OAEP padding. | RsaWithoutOaepTest.java:11:35:11:38 | spec | initialize an RSA cipher |
3+
| RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | This specification is used to $@ without OAEP padding. | RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | initialize an RSA cipher |
4+
| RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:13:35:13:38 | spec | This specification is used to $@ without OAEP padding. | RsaWithoutOaepTest.java:13:35:13:38 | spec | initialize an RSA cipher |
45
edges
5-
| RsaWithoutOaepTest.java:10:29:10:39 | spec : String | RsaWithoutOaepTest.java:11:35:11:38 | spec | provenance | |
6-
| RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:10:29:10:39 | spec : String | provenance | |
6+
| RsaWithoutOaepTest.java:12:29:12:39 | spec : String | RsaWithoutOaepTest.java:13:35:13:38 | spec | provenance | |
7+
| RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:12:29:12:39 | spec : String | provenance | |
78
nodes
89
| RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | semmle.label | "RSA/ECB/NoPadding" |
9-
| RsaWithoutOaepTest.java:10:29:10:39 | spec : String | semmle.label | spec : String |
10-
| RsaWithoutOaepTest.java:11:35:11:38 | spec | semmle.label | spec |
11-
| RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | semmle.label | "RSA/ECB/NoPadding" : String |
10+
| RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | semmle.label | "rsa/ECB/NoPadding" |
11+
| RsaWithoutOaepTest.java:12:29:12:39 | spec : String | semmle.label | spec : String |
12+
| RsaWithoutOaepTest.java:13:35:13:38 | spec | semmle.label | spec |
13+
| RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | semmle.label | "RSA/ECB/NoPadding" : String |
1214
subpaths

‎java/ql/test/query-tests/security/CWE-780/RsaWithoutOaepTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
class RsaWithoutOaep {
44
public void test() throws Exception {
55
Cipher rsaBad = Cipher.getInstance("RSA/ECB/NoPadding"); // $ Alert
6+
Cipher rsaBadLower = Cipher.getInstance("rsa/ECB/NoPadding"); // $ Alert
67

78
Cipher rsaGood = Cipher.getInstance("RSA/ECB/OAEPWithSHA-1AndMGF1Padding");
9+
Cipher rsaGoodLower = Cipher.getInstance("rsa/ecb/oaepwithsha-1andmgf1padding");
810
}
911

1012
public Cipher getCipher(String spec) throws Exception {

0 commit comments

Comments
 (0)