From 26f4abf12b6f706ca3d98b6d844bec894e2715b1 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Tue, 11 Oct 2022 16:31:02 -0400 Subject: [PATCH] remove globalflow for key(pair)gen --- .../security/InsufficientKeySizeQuery.qll | 124 +++++++++--------- .../CWE-326/InsufficientKeySizeTest.java | 4 +- 2 files changed, 64 insertions(+), 64 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index 2891bbcac50..c6dba3959cc 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -15,17 +15,19 @@ class AsymmetricNonECKeyTrackingConfiguration extends DataFlow2::Configuration { } override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma | + exists(MethodAccess ma, JavaSecurityKeyPairGenerator jpg | ma.getMethod() instanceof KeyPairGeneratorInitMethod and - exists( - JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig, - DataFlow::PathNode source, DataFlow::PathNode dest - | - jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and - source.getNode().asExpr() = jpg and - dest.getNode().asExpr() = ma.getQualifier() and - kpgConfig.hasFlowPath(source, dest) - ) and + jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and + DataFlow::localExprFlow(jpg, ma.getQualifier()) and + // exists( + // JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig, + // DataFlow::PathNode source, DataFlow::PathNode dest + // | + // jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and + // source.getNode().asExpr() = jpg and + // dest.getNode().asExpr() = ma.getQualifier() and + // kpgConfig.hasFlowPath(source, dest) + // ) and sink.asExpr() = ma.getArgument(0) ) or @@ -59,17 +61,19 @@ class AsymmetricECKeyTrackingConfiguration extends DataFlow2::Configuration { } override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma | + exists(MethodAccess ma, JavaSecurityKeyPairGenerator jpg | ma.getMethod() instanceof KeyPairGeneratorInitMethod and - exists( - JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig, - DataFlow::PathNode source, DataFlow::PathNode dest - | - jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and - source.getNode().asExpr() = jpg and - dest.getNode().asExpr() = ma.getQualifier() and - kpgConfig.hasFlowPath(source, dest) - ) and + jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and + DataFlow::localExprFlow(jpg, ma.getQualifier()) and + // exists( + // JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig, + // DataFlow::PathNode source, DataFlow::PathNode dest + // | + // jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and + // source.getNode().asExpr() = jpg and + // dest.getNode().asExpr() = ma.getQualifier() and + // kpgConfig.hasFlowPath(source, dest) + // ) and sink.asExpr() = ma.getArgument(0) ) or @@ -92,17 +96,19 @@ class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration { } override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma | + exists(MethodAccess ma, JavaxCryptoKeyGenerator jcg | ma.getMethod() instanceof KeyGeneratorInitMethod and - exists( - JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig, - DataFlow::PathNode source, DataFlow::PathNode dest - | - jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and - source.getNode().asExpr() = jcg and - dest.getNode().asExpr() = ma.getQualifier() and - kgConfig.hasFlowPath(source, dest) - ) and + jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and + DataFlow::localExprFlow(jcg, ma.getQualifier()) and + // exists( + // JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig, + // DataFlow::PathNode source, DataFlow::PathNode dest + // | + // jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and + // source.getNode().asExpr() = jcg and + // dest.getNode().asExpr() = ma.getQualifier() and + // kgConfig.hasFlowPath(source, dest) + // ) and sink.asExpr() = ma.getArgument(0) ) } @@ -110,38 +116,32 @@ class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration { // ********************** Need the below models for the above configs ********************** // todo: move some/all of below to Encryption.qll or elsewhere? -/** A data flow configuration tracking flow from a key generator to an `init` method call. */ -private class KeyGeneratorInitConfiguration extends DataFlow::Configuration { - KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" } - - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof JavaxCryptoKeyGenerator - } - - override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma | - ma.getMethod() instanceof KeyGeneratorInitMethod and - sink.asExpr() = ma.getQualifier() - ) - } -} - -/** A data flow configuration tracking flow from a keypair generator to an `initialize` method call. */ -private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration { - KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" } - - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof JavaSecurityKeyPairGenerator - } - - override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess ma | - ma.getMethod() instanceof KeyPairGeneratorInitMethod and - sink.asExpr() = ma.getQualifier() - ) - } -} - +// /** A data flow configuration tracking flow from a key generator to an `init` method call. */ +// private class KeyGeneratorInitConfiguration extends DataFlow::Configuration { +// KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" } +// override predicate isSource(DataFlow::Node source) { +// source.asExpr() instanceof JavaxCryptoKeyGenerator +// } +// override predicate isSink(DataFlow::Node sink) { +// exists(MethodAccess ma | +// ma.getMethod() instanceof KeyGeneratorInitMethod and +// sink.asExpr() = ma.getQualifier() +// ) +// } +// } +// /** A data flow configuration tracking flow from a keypair generator to an `initialize` method call. */ +// private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration { +// KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" } +// override predicate isSource(DataFlow::Node source) { +// source.asExpr() instanceof JavaSecurityKeyPairGenerator +// } +// override predicate isSink(DataFlow::Node sink) { +// exists(MethodAccess ma | +// ma.getMethod() instanceof KeyPairGeneratorInitMethod and +// sink.asExpr() = ma.getQualifier() +// ) +// } +// } /** The Java class `java.security.spec.ECGenParameterSpec`. */ private class EcGenParameterSpec extends RefType { EcGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") } 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 9ba11f1c632..e6a61d7bb4c 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java @@ -208,7 +208,7 @@ public class InsufficientKeySizeTest { keyGen.init(keySize); // $ hasInsufficientKeySize // BAD: Key size is less than 2048 - kg.init(64); // $ hasInsufficientKeySize + kg.init(64); // $ MISSING: hasInsufficientKeySize } //! refactor this to use expected-value tag and combine with above method @@ -224,7 +224,7 @@ public class InsufficientKeySizeTest { keyPairGen.initialize(keySize); // $ hasInsufficientKeySize // BAD: Key size is less than 2048 - kpg.initialize(1024); // $ hasInsufficientKeySize + kpg.initialize(1024); // $ MISSING: hasInsufficientKeySize } //! refactor this to use expected-value tag and combine with above method