From 75794ec7a7041c8ae3d5774c1d7fe601b2f4efe2 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Tue, 4 Oct 2022 11:59:20 -0400 Subject: [PATCH] false negative testing - before rewrite for variable dataflow --- .../security/InsufficientKeySizeQuery.qll | 14 +++++++++-- .../CWE-326/InsufficientKeySizeTest.java | 23 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index eaba8977c23..d6943adebc5 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -1,5 +1,6 @@ import semmle.code.java.security.Encryption import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.DataFlow3 /** The Java class `java.security.spec.ECGenParameterSpec`. */ private class ECGenParameterSpec extends RefType { @@ -86,9 +87,18 @@ private predicate hasShortSymmetricKey(MethodAccess ma, string msg, string type) jcg.getAlgoSpec().(StringLiteral).getValue() = type and source.getNode().asExpr() = jcg and dest.getNode().asExpr() = ma.getQualifier() and - cc.hasFlowPath(source, dest) + //ma.getArgument(0) = var and // ! me + //var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and // ! me + cc.hasFlowPath(source, dest) //and + //var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 128 // ! me + ) and + exists(VarAccess var | + var.getVariable().getInitializer().getUnderlyingExpr() instanceof IntegerLiteral and + var.getVariable().getInitializer().getUnderlyingExpr().toString().toInt() < 128 and + //DataFlow3::localExprFlow(var, ma.getArgument(0)) and + ma.getArgument(0) = var + //ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 ) and - ma.getArgument(0).(IntegerLiteral).getIntValue() < 128 and msg = "Key size should be at least 128 bits for " + type + " encryption." } diff --git a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java index fbff6187855..d070eaf3d76 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java @@ -3,7 +3,7 @@ import java.security.spec.ECGenParameterSpec; import javax.crypto.KeyGenerator; public class InsufficientKeySizeTest { - public void CryptoMethod() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + public void cryptoMethod() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { KeyGenerator keyGen1 = KeyGenerator.getInstance("AES"); // BAD: Key size is less than 128 keyGen1.init(64); // $ hasInsufficientKeySize @@ -89,5 +89,26 @@ public class InsufficientKeySizeTest { KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); // GOOD: Key size is no less than 2048 keyPairGen17.initialize(2048); // Safe + + + // FN: Test with variables as numbers + final int size1 = 64; + KeyGenerator keyGen3 = KeyGenerator.getInstance("AES"); + // BAD: Key size is less than 128 + keyGen3.init(size1); // $ hasInsufficientKeySize + + int size2 = 1024; + KeyPairGenerator keyPairGen18 = KeyPairGenerator.getInstance("RSA"); + // BAD: Key size is less than 2048 + keyPairGen18.initialize(size2); // $ hasInsufficientKeySize + + int keysize = 64; + test(keysize); + } + + public void test(int keySize) throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { + KeyGenerator keyGen4 = KeyGenerator.getInstance("AES"); + // BAD: Key size is less than 128 + keyGen4.init(keySize); // $ hasInsufficientKeySize } }