Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* 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`).
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class BrokenAlgoLiteral extends ShortStringLiteral {
BrokenAlgoLiteral() {
this.getValue().regexpMatch(getInsecureAlgorithmRegex()) and
// Exclude RSA/ECB/.* ciphers.
not this.getValue().regexpMatch("RSA/ECB.*") and
not this.getValue().regexpMatch("(?i)RSA/ECB.*") and
// Exclude German and French sentences.
not this.getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
}
Expand Down
9 changes: 6 additions & 3 deletions java/ql/lib/semmle/code/java/security/Encryption.qll
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,18 @@ bindingset[algorithmString]
private string algorithmRegex(string algorithmString) {
// Algorithms usually appear in names surrounded by characters that are not
// alphabetical characters in the same case. This handles the upper and lower
// case cases.
// case cases, plus mixed-case algorithm strings bounded by non-alphabetic
// characters (such as `AES/cbc/Pkcs5Padding`).
result =
"((^|.*[^A-Z])(" + algorithmString + ")([^A-Z].*|$))" +
// or...
"|" +
// For lowercase, we want to be careful to avoid being confused by camelCase
// hence we require two preceding uppercase letters to be sure of a case switch,
// or a preceding non-alphabetic character
"((^|.*[A-Z]{2}|.*[^a-zA-Z])(" + algorithmString.toLowerCase() + ")([^a-z].*|$))"
"((^|.*[A-Z]{2}|.*[^a-zA-Z])(" + algorithmString.toLowerCase() + ")([^a-z].*|$))" +
// or...
"|" + "((^|.*[^a-zA-Z])(?i)(" + algorithmString + ")([^a-zA-Z].*|$))"
}

/**
Expand Down Expand Up @@ -258,7 +261,7 @@ string getInsecureAlgorithmReason(string input) {
string getASecureAlgorithmName() {
result =
[
"RSA", "SHA-?(256|384|512)", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))",
"RSA", "SHA-?(256|384|512)", "CCM", "GCM", "AES(?![^a-zA-Z](?i)(ECB|CBC/PKCS[57]Padding))",
"Blowfish", "ECIES", "SHA3-(256|384|512)",
// Elliptic Curve algorithms: EC (key generation), ECDSA (signatures), ECDH (key agreement),
// EdDSA/Ed25519/Ed448 (Edwards-curve signatures), XDH/X25519/X448 (key agreement).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import semmle.code.java.dataflow.DataFlow
module RsaWithoutOaepConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
exists(CompileTimeConstantExpr specExpr, string spec |
specExpr.getStringValue() = spec and
specExpr.getStringValue().toUpperCase() = spec and
specExpr = src.asExpr() and
spec.matches("RSA/%") and
not spec.matches("%OAEP%")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
edges
nodes
| Test.java:19:45:19:49 | "DES" | semmle.label | "DES" |
| Test.java:42:33:42:37 | "RC2" | semmle.label | "RC2" |
| Test.java:46:33:46:54 | "AES/CBC/PKCS5Padding" | semmle.label | "AES/CBC/PKCS5Padding" |
| Test.java:47:33:47:54 | "AES/CBC/PKCS5PADDING" | semmle.label | "AES/CBC/PKCS5PADDING" |
| Test.java:48:33:48:54 | "aes/cbc/pkcs5padding" | semmle.label | "aes/cbc/pkcs5padding" |
| Test.java:49:33:49:54 | "AES/cbc/Pkcs5Padding" | semmle.label | "AES/cbc/Pkcs5Padding" |
subpaths
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ public void test() {
// BAD: RC2 is a weak algorithm
cipher = Cipher.getInstance("RC2"); // $ Alert[java/weak-cryptographic-algorithm]
}
{
// BAD: AES/CBC/PKCS5Padding is vulnerable to padding oracle attacks, regardless of case
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); // $ Alert[java/weak-cryptographic-algorithm]
cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING"); // $ Alert[java/weak-cryptographic-algorithm]
cipher = Cipher.getInstance("aes/cbc/pkcs5padding"); // $ Alert[java/weak-cryptographic-algorithm]
cipher = Cipher.getInstance("AES/cbc/Pkcs5Padding"); // $ Alert[java/weak-cryptographic-algorithm]
}
// GOOD: ECIES is a strong algorithm
cipher = Cipher.getInstance("ECIES");
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#select
| 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 |
| 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 |
| 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 |
| 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 |
edges
| RsaWithoutOaepTest.java:10:29:10:39 | spec : String | RsaWithoutOaepTest.java:11:35:11:38 | spec | provenance | |
| RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:10:29:10:39 | spec : String | provenance | |
| RsaWithoutOaepTest.java:12:29:12:39 | spec : String | RsaWithoutOaepTest.java:13:35:13:38 | spec | provenance | |
| RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | RsaWithoutOaepTest.java:12:29:12:39 | spec : String | provenance | |
nodes
| RsaWithoutOaepTest.java:5:44:5:62 | "RSA/ECB/NoPadding" | semmle.label | "RSA/ECB/NoPadding" |
| RsaWithoutOaepTest.java:10:29:10:39 | spec : String | semmle.label | spec : String |
| RsaWithoutOaepTest.java:11:35:11:38 | spec | semmle.label | spec |
| RsaWithoutOaepTest.java:15:32:15:50 | "RSA/ECB/NoPadding" : String | semmle.label | "RSA/ECB/NoPadding" : String |
| RsaWithoutOaepTest.java:6:49:6:67 | "rsa/ECB/NoPadding" | semmle.label | "rsa/ECB/NoPadding" |
| RsaWithoutOaepTest.java:12:29:12:39 | spec : String | semmle.label | spec : String |
| RsaWithoutOaepTest.java:13:35:13:38 | spec | semmle.label | spec |
| RsaWithoutOaepTest.java:17:32:17:50 | "RSA/ECB/NoPadding" : String | semmle.label | "RSA/ECB/NoPadding" : String |
subpaths
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
class RsaWithoutOaep {
public void test() throws Exception {
Cipher rsaBad = Cipher.getInstance("RSA/ECB/NoPadding"); // $ Alert
Cipher rsaBadLower = Cipher.getInstance("rsa/ECB/NoPadding"); // $ Alert

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

public Cipher getCipher(String spec) throws Exception {
Expand Down
Loading