Merge pull request #4926 from luchua-bc/java/insufficient-key-size

Java: Query to detect weak encryption: insufficient key size

java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java

@@ -0,0 +1,37 @@
+public class InsufficientKeySize {
+    public void CryptoMethod() {
+        KeyGenerator keyGen1 = KeyGenerator.getInstance("AES");
+        // BAD: Key size is less than 128
+        keyGen1.init(64);
+
+        KeyGenerator keyGen2 = KeyGenerator.getInstance("AES");
+        // GOOD: Key size is no less than 128
+        keyGen2.init(128);
+
+        KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA");
+        // BAD: Key size is less than 2048
+        keyPairGen1.initialize(1024);
+
+        KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA");
+        // GOOD: Key size is no less than 2048
+        keyPairGen2.initialize(2048);
+
+        KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA");
+        // BAD: Key size is less than 2048
+        keyPairGen3.initialize(1024);
+
+        KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA");
+        // GOOD: Key size is no less than 2048
+        keyPairGen4.initialize(2048);
+
+        KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1");
+        keyPairGen5.initialize(ecSpec1);
+
+        KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC");
+        // GOOD: Key size is no less than 256
+        ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1");
+        keyPairGen6.initialize(ecSpec2);
+    }
+}

java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>This rule finds uses of encryption algorithms with too small a key size. Encryption algorithms 
+are vulnerable to brute force attack when too small a key size is used.</p>
+    </overview>
+
+    <recommendation>
+        <p>The key should be at least 2048 bits long when using RSA and DSA encryption, 256 bits long when using EC encryption, and 128 bits long when using 
+symmetric encryption.</p>
+    </recommendation>
+    
+    <references>
+
+        <li>
+            Wikipedia.
+            <a href="http://en.wikipedia.org/wiki/Key_size">Key size</a>
+        </li>
+        <li>
+            SonarSource.
+            <a href="https://rules.sonarsource.com/java/type/Vulnerability/RSPEC-4426">Cryptographic keys should be robust</a>
+        </li>
+        <li>
+            CWE.
+            <a href="https://cwe.mitre.org/data/definitions/326.html">CWE-326: Inadequate Encryption Strength</a>
+        </li>
+ 
+    </references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql

@@ -0,0 +1,151 @@
+/**
+ * @name Weak encryption: Insufficient key size
+ * @description Finds uses of encryption algorithms with too small a key size
+ * @kind problem
+ * @id java/insufficient-key-size
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import java
+import semmle.code.java.security.Encryption
+import semmle.code.java.dataflow.TaintTracking
+
+/** The Java class `java.security.spec.ECGenParameterSpec`. */
+class ECGenParameterSpec extends RefType {
+  ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
+}
+
+/** The `init` method declared in `javax.crypto.KeyGenerator`. */
+class KeyGeneratorInitMethod extends Method {
+  KeyGeneratorInitMethod() {
+    getDeclaringType() instanceof KeyGenerator and
+    hasName("init")
+  }
+}
+
+/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
+class KeyPairGeneratorInitMethod extends Method {
+  KeyPairGeneratorInitMethod() {
+    getDeclaringType() instanceof KeyPairGenerator and
+    hasName("initialize")
+  }
+}
+
+/** Returns the key size in the EC algorithm string */
+bindingset[algorithm]
+int getECKeySize(string algorithm) {
+  algorithm.matches("sec%") and // specification such as "secp256r1"
+  result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
+  or
+  algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2"
+  result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
+  or
+  (algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2"
+  result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
+}
+
+/** Taint configuration tracking flow from a key generator to a `init` method call. */
+class KeyGeneratorInitConfiguration extends TaintTracking::Configuration {
+  KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(JavaxCryptoKeyGenerator jcg | jcg = source.asExpr())
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof KeyGeneratorInitMethod and
+      sink.asExpr() = ma.getQualifier()
+    )
+  }
+}
+
+/** Taint configuration tracking flow from a keypair generator to a `initialize` method call. */
+class KeyPairGeneratorInitConfiguration extends TaintTracking::Configuration {
+  KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(JavaSecurityKeyPairGenerator jkg | jkg = source.asExpr())
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof KeyPairGeneratorInitMethod and
+      sink.asExpr() = ma.getQualifier()
+    )
+  }
+}
+
+/** Holds if a symmetric `KeyGenerator` implementing encryption algorithm `type` and initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+bindingset[type]
+predicate hasShortSymmetricKey(MethodAccess ma, string msg, string type) {
+  ma.getMethod() instanceof KeyGeneratorInitMethod and
+  exists(
+    JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration cc, DataFlow::PathNode source,
+    DataFlow::PathNode dest
+  |
+    jcg.getAlgoSpec().(StringLiteral).getValue() = type and
+    source.getNode().asExpr() = jcg and
+    dest.getNode().asExpr() = ma.getQualifier() and
+    cc.hasFlowPath(source, dest)
+  ) and
+  ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 and
+  msg = "Key size should be at least 128 bits for " + type + " encryption."
+}
+
+/** Holds if an AES `KeyGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+predicate hasShortAESKey(MethodAccess ma, string msg) { hasShortSymmetricKey(ma, msg, "AES") }
+
+/** Holds if an asymmetric `KeyPairGenerator` implementing encryption algorithm `type` and initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+bindingset[type]
+predicate hasShortAsymmetricKeyPair(MethodAccess ma, string msg, string type) {
+  ma.getMethod() instanceof KeyPairGeneratorInitMethod and
+  exists(
+    JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc,
+    DataFlow::PathNode source, DataFlow::PathNode dest
+  |
+    jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = type and
+    source.getNode().asExpr() = jpg and
+    dest.getNode().asExpr() = ma.getQualifier() and
+    kc.hasFlowPath(source, dest)
+  ) and
+  ma.getArgument(0).(IntegerLiteral).getIntValue() < 2048 and
+  msg = "Key size should be at least 2048 bits for " + type + " encryption."
+}
+
+/** Holds if a DSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+predicate hasShortDSAKeyPair(MethodAccess ma, string msg) {
+  hasShortAsymmetricKeyPair(ma, msg, "DSA") or hasShortAsymmetricKeyPair(ma, msg, "DH")
+}
+
+/** Holds if a RSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+predicate hasShortRSAKeyPair(MethodAccess ma, string msg) {
+  hasShortAsymmetricKeyPair(ma, msg, "RSA")
+}
+
+/** Holds if an EC `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
+predicate hasShortECKeyPair(MethodAccess ma, string msg) {
+  ma.getMethod() instanceof KeyPairGeneratorInitMethod and
+  exists(
+    JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kc,
+    DataFlow::PathNode source, DataFlow::PathNode dest, ClassInstanceExpr cie
+  |
+    jpg.getAlgoSpec().(StringLiteral).getValue().matches("EC%") and // ECC variants such as ECDH and ECDSA
+    source.getNode().asExpr() = jpg and
+    dest.getNode().asExpr() = ma.getQualifier() and
+    kc.hasFlowPath(source, dest) and
+    DataFlow::localExprFlow(cie, ma.getArgument(0)) and
+    ma.getArgument(0).getType() instanceof ECGenParameterSpec and
+    getECKeySize(cie.getArgument(0).(StringLiteral).getRepresentedString()) < 256
+  ) and
+  msg = "Key size should be at least 256 bits for EC encryption."
+}
+
+from Expr e, string msg
+where
+  hasShortAESKey(e, msg) or
+  hasShortDSAKeyPair(e, msg) or
+  hasShortRSAKeyPair(e, msg) or
+  hasShortECKeyPair(e, msg)
+select e, msg

java/ql/src/semmle/code/java/security/Encryption.qll

@@ -38,6 +38,16 @@ class HostnameVerifier extends RefType {
   HostnameVerifier() { hasQualifiedName("javax.net.ssl", "HostnameVerifier") }
 }
 
+/** The Java class `javax.crypto.KeyGenerator`. */
+class KeyGenerator extends RefType {
+  KeyGenerator() { this.hasQualifiedName("javax.crypto", "KeyGenerator") }
+}
+
+/** The Java class `java.security.KeyPairGenerator`. */
+class KeyPairGenerator extends RefType {
+  KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") }
+}
+
 /** The `verify` method of the class `javax.net.ssl.HostnameVerifier`. */
 class HostnameVerifierVerify extends Method {
   HostnameVerifierVerify() {
@@ -248,7 +258,7 @@ class JavaxCryptoSecretKey extends JavaxCryptoAlgoSpec {
 class JavaxCryptoKeyGenerator extends JavaxCryptoAlgoSpec {
   JavaxCryptoKeyGenerator() {
     exists(Method m | m.getAReference() = this |
-      m.getDeclaringType().getQualifiedName() = "javax.crypto.KeyGenerator" and
+      m.getDeclaringType() instanceof KeyGenerator and
       m.getName() = "getInstance"
     )
   }
@@ -304,3 +314,15 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec {
 
   override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
 }
+
+/** A method call to the Java class `java.security.KeyPairGenerator`. */
+class JavaSecurityKeyPairGenerator extends JavaxCryptoAlgoSpec {
+  JavaSecurityKeyPairGenerator() {
+    exists(Method m | m.getAReference() = this |
+      m.getDeclaringType() instanceof KeyPairGenerator and
+      m.getName() = "getInstance"
+    )
+  }
+
+  override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
+}

java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected

@@ -0,0 +1,11 @@
+| InsufficientKeySize.java:9:9:9:24 | init(...) | Key size should be at least 128 bits for AES encryption. |
+| InsufficientKeySize.java:17:9:17:36 | initialize(...) | Key size should be at least 2048 bits for RSA encryption. |
+| InsufficientKeySize.java:25:9:25:36 | initialize(...) | Key size should be at least 2048 bits for DSA encryption. |
+| InsufficientKeySize.java:34:9:34:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:38:9:38:67 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:48:9:48:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:53:9:53:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:58:9:58:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:68:9:68:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:78:9:78:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
+| InsufficientKeySize.java:87:9:87:37 | initialize(...) | Key size should be at least 2048 bits for DH encryption. |

java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java

@@ -0,0 +1,93 @@
+import java.security.KeyPairGenerator;
+import java.security.spec.ECGenParameterSpec;
+import javax.crypto.KeyGenerator;
+
+public class InsufficientKeySize {
+    public void CryptoMethod() {
+        KeyGenerator keyGen1 = KeyGenerator.getInstance("AES");
+        // BAD: Key size is less than 128
+        keyGen1.init(64);
+
+        KeyGenerator keyGen2 = KeyGenerator.getInstance("AES");
+        // GOOD: Key size is no less than 128
+        keyGen2.init(128);
+
+        KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA");
+        // BAD: Key size is less than 2048
+        keyPairGen1.initialize(1024);
+
+        KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA");
+        // GOOD: Key size is no less than 2048
+        keyPairGen2.initialize(2048);
+
+        KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA");
+        // BAD: Key size is less than 2048
+        keyPairGen3.initialize(1024);
+
+        KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA");
+        // GOOD: Key size is no less than 2048
+        keyPairGen4.initialize(2048);
+
+        KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1");
+        keyPairGen5.initialize(ecSpec1);
+
+        KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        keyPairGen6.initialize(new ECGenParameterSpec("secp112r1"));
+
+        KeyPairGenerator keyPairGen7 = KeyPairGenerator.getInstance("EC");
+        // GOOD: Key size is no less than 256
+        ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1");
+        keyPairGen7.initialize(ecSpec2);
+
+        KeyPairGenerator keyPairGen8 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec3 = new ECGenParameterSpec("X9.62 prime192v2");
+        keyPairGen8.initialize(ecSpec3);
+
+        KeyPairGenerator keyPairGen9 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec4 = new ECGenParameterSpec("X9.62 c2tnb191v3");
+        keyPairGen9.initialize(ecSpec4);
+
+        KeyPairGenerator keyPairGen10 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec5 = new ECGenParameterSpec("sect163k1");
+        keyPairGen10.initialize(ecSpec5);
+
+        KeyPairGenerator keyPairGen11 = KeyPairGenerator.getInstance("EC");
+        // GOOD: Key size is no less than 256
+        ECGenParameterSpec ecSpec6 = new ECGenParameterSpec("X9.62 c2tnb359v1");
+        keyPairGen11.initialize(ecSpec6);
+
+        KeyPairGenerator keyPairGen12 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec7 = new ECGenParameterSpec("prime192v2");
+        keyPairGen12.initialize(ecSpec7);
+
+        KeyPairGenerator keyPairGen13 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is no less than 256
+        ECGenParameterSpec ecSpec8 = new ECGenParameterSpec("prime256v1");
+        keyPairGen13.initialize(ecSpec8);
+
+        KeyPairGenerator keyPairGen14 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is less than 256
+        ECGenParameterSpec ecSpec9 = new ECGenParameterSpec("c2tnb191v1");
+        keyPairGen14.initialize(ecSpec9);
+
+        KeyPairGenerator keyPairGen15 = KeyPairGenerator.getInstance("EC");
+        // BAD: Key size is no less than 256
+        ECGenParameterSpec ecSpec10 = new ECGenParameterSpec("c2tnb431r1");
+        keyPairGen15.initialize(ecSpec10);
+
+        KeyPairGenerator keyPairGen16 = KeyPairGenerator.getInstance("dh");
+        // BAD: Key size is less than 2048
+        keyPairGen16.initialize(1024);
+
+        KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH");
+        // GOOD: Key size is no less than 2048
+        keyPairGen17.initialize(2048);
+    }
+}

java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-326/InsufficientKeySize.ql