From cfc950f8039ca780b87a6889b2913e6702961b4e Mon Sep 17 00:00:00 2001 From: luchua-bc Date: Thu, 7 Jan 2021 21:18:51 +0000 Subject: [PATCH] Query for weak encryption: Insufficient key size --- .../CWE/CWE-326/InsufficientKeySize.java | 37 +++++ .../CWE/CWE-326/InsufficientKeySize.qhelp | 33 ++++ .../CWE/CWE-326/InsufficientKeySize.ql | 155 ++++++++++++++++++ .../semmle/code/java/security/Encryption.qll | 12 ++ .../CWE-326/InsufficientKeySize.expected | 4 + .../security/CWE-326/InsufficientKeySize.java | 41 +++++ .../CWE-326/InsufficientKeySize.qlref | 1 + 7 files changed, 283 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java create mode 100644 java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql create mode 100644 java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java create mode 100644 java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java b/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.java new file mode 100644 index 00000000000..6ccf025c244 --- /dev/null +++ b/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 224 + ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); + keyPairGen5.initialize(ecSpec1); + + KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); + // GOOD: Key size is no less than 224 + ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); + keyPairGen6.initialize(ecSpec2); + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp b/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp new file mode 100644 index 00000000000..8de21f1c90a --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.qhelp @@ -0,0 +1,33 @@ + + + +

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.

+ + + +

The key should be at least 2048-bit long when using RSA and DSA encryption, 224-bit long when using EC encryption, and 128-bit long when using +symmetric encryption.

+ + + + +
  • + Wikipedia. + Key size +
    • +
  • + SonarSource. + Cryptographic keys should be robust +
    • +
  • + CWE. + CWE-326: Inadequate Encryption Strength +
    • +
  • + C# implementation of CodeQL + codeql/csharp/ql/src/Security Features/InsufficientKeySize.ql +
    • + +     +     \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql b/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql new file mode 100644 index 00000000000..882d997f1b8 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-326/InsufficientKeySize.ql @@ -0,0 +1,155 @@ +/** + * @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 `javax.crypto.KeyGenerator`. */ +class KeyGenerator extends RefType { + KeyGenerator() { this.hasQualifiedName("javax.crypto", "KeyGenerator") } +} + +/** The Java class `javax.crypto.KeyGenerator`. */ +class KeyPairGenerator extends RefType { + KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") } +} + +/** 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") + } +} + +/** Taint configuration tracking flow from a key generator to a `init` method call. */ +class CryptoKeyGeneratorConfiguration extends TaintTracking::Configuration { + CryptoKeyGeneratorConfiguration() { this = "CryptoKeyGeneratorConfiguration" } + + 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 KeyPairGeneratorConfiguration extends TaintTracking::Configuration { + KeyPairGeneratorConfiguration() { this = "KeyPairGeneratorConfiguration" } + + 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 an AES `KeyGenerator` is initialized with an insufficient key size. */ +predicate incorrectUseOfAES(MethodAccess ma, string msg) { + ma.getMethod() instanceof KeyGeneratorInitMethod and + exists( + JavaxCryptoKeyGenerator jcg, CryptoKeyGeneratorConfiguration cc, DataFlow::PathNode source, + DataFlow::PathNode dest + | + jcg.getAlgoSpec().(StringLiteral).getValue() = "AES" 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 AES encryption." +} + +/** Holds if a DSA `KeyPairGenerator` is initialized with an insufficient key size. */ +predicate incorrectUseOfDSA(MethodAccess ma, string msg) { + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + exists( + JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorConfiguration kc, DataFlow::PathNode source, + DataFlow::PathNode dest + | + jpg.getAlgoSpec().(StringLiteral).getValue() = "DSA" 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 DSA encryption." +} + +/** Holds if a RSA `KeyPairGenerator` is initialized with an insufficient key size. */ +predicate incorrectUseOfRSA(MethodAccess ma, string msg) { + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + exists( + JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorConfiguration kc, DataFlow::PathNode source, + DataFlow::PathNode dest + | + jpg.getAlgoSpec().(StringLiteral).getValue() = "RSA" 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 RSA encryption." +} + +/** Holds if an EC `KeyPairGenerator` is initialized with an insufficient key size. */ +predicate incorrectUseOfEC(MethodAccess ma, string msg) { + ma.getMethod() instanceof KeyPairGeneratorInitMethod and + exists( + JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorConfiguration 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 + exists(VariableAssign va | + ma.getArgument(0).(VarAccess).getVariable() = va.getDestVar() and + va.getSource() = cie and + cie.getArgument(0) + .(StringLiteral) + .getRepresentedString() + .regexpCapture(".*[a-zA-Z]+([0-9]+)[a-zA-Z]+.*", 1) + .toInt() < 224 + ) + ) and + msg = "Key size should be at least 224 bits for EC encryption." +} + +from Expr e, string msg +where + incorrectUseOfAES(e, msg) or + incorrectUseOfDSA(e, msg) or + incorrectUseOfRSA(e, msg) or + incorrectUseOfEC(e, msg) +select e, msg diff --git a/java/ql/src/semmle/code/java/security/Encryption.qll b/java/ql/src/semmle/code/java/security/Encryption.qll index d48a5c6c715..b133fd3c523 100644 --- a/java/ql/src/semmle/code/java/security/Encryption.qll +++ b/java/ql/src/semmle/code/java/security/Encryption.qll @@ -304,3 +304,15 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec { override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) } } + +/** Method call to the Java class `java.security.KeyPairGenerator`. */ +class JavaSecurityKeyPairGenerator extends JavaxCryptoAlgoSpec { + JavaSecurityKeyPairGenerator() { + exists(Method m | m.getAReference() = this | + m.getDeclaringType().getQualifiedName() = "java.security.KeyPairGenerator" and + m.getName() = "getInstance" + ) + } + + override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected new file mode 100644 index 00000000000..b47469aed5d --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.expected @@ -0,0 +1,4 @@ +| 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 224 bits for EC encryption. | diff --git a/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java new file mode 100644 index 00000000000..13f74b6fac3 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.java @@ -0,0 +1,41 @@ +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 224 + ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1"); + keyPairGen5.initialize(ecSpec1); + + KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC"); + // GOOD: Key size is no less than 224 + ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1"); + keyPairGen6.initialize(ecSpec2); + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref new file mode 100644 index 00000000000..2b35cd6921e --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-326/InsufficientKeySize.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-326/InsufficientKeySize.ql \ No newline at end of file