diff --git a/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll index ee87fdc64ee..979c0c9c149 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll @@ -6,9 +6,11 @@ import semmle.code.java.security.UnsafeCertTrust import semmle.code.java.security.Encryption /** + * DEPRECATED: Use `SslEndpointIdentificationFlow` instead. + * * A taint flow configuration for SSL connections created without a proper certificate trust configuration. */ -class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { +deprecated class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" } override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit } @@ -20,30 +22,38 @@ class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { } } +private module SslEndpointIdentificationFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit } + + predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof SslUnsafeCertTrustSanitizer } +} + +module SslEndpointIdentificationFlow = TaintTracking::Global; + /** * An SSL object that was assigned a safe `SSLParameters` object and can be considered safe. */ private class SslConnectionWithSafeSslParameters extends SslUnsafeCertTrustSanitizer { SslConnectionWithSafeSslParameters() { - exists(SafeSslParametersFlowConfig config, DataFlow::Node safe, DataFlow::Node sanitizer | - config.hasFlowTo(safe) and + exists(DataFlow::Node safe, DataFlow::Node sanitizer | + SafeSslParametersFlow::flowTo(safe) and sanitizer = DataFlow::exprNode(safe.asExpr().(Argument).getCall().getQualifier()) and DataFlow::localFlow(sanitizer, this) ) } } -private class SafeSslParametersFlowConfig extends DataFlow2::Configuration { - SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" } - - override predicate isSource(DataFlow::Node source) { +private module SafeSslParametersFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(MethodAccess ma | ma instanceof SafeSetEndpointIdentificationAlgorithm and DataFlow::getInstanceArgument(ma) = source.(DataFlow::PostUpdateNode).getPreUpdateNode() ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma, RefType t | t instanceof SslSocket or t instanceof SslEngine | ma.getMethod().hasName("setSSLParameters") and ma.getMethod().getDeclaringType().getAnAncestor() = t and @@ -52,6 +62,8 @@ private class SafeSslParametersFlowConfig extends DataFlow2::Configuration { } } +private module SafeSslParametersFlow = DataFlow::Global; + /** * A call to `SSLParameters.setEndpointIdentificationAlgorithm` with a non-null and non-empty parameter. */ diff --git a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql index 2925d588c74..eb5241ac87a 100644 --- a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql +++ b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql @@ -18,7 +18,5 @@ import semmle.code.java.security.UnsafeCertTrustQuery from Expr unsafeTrust where unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet or - exists(SslEndpointIdentificationFlowConfig config | - config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) - ) + SslEndpointIdentificationFlow::flowTo(DataFlow::exprNode(unsafeTrust)) select unsafeTrust, "Unsafe configuration of trusted certificates." diff --git a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql index 344faa7f86b..fd18ccc27eb 100644 --- a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql +++ b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql @@ -12,9 +12,7 @@ class UnsafeCertTrustTest extends InlineExpectationsTest { exists(Expr unsafeTrust | unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet or - exists(SslEndpointIdentificationFlowConfig config | - config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) - ) + SslEndpointIdentificationFlow::flowTo(DataFlow::exprNode(unsafeTrust)) | unsafeTrust.getLocation() = location and element = unsafeTrust.toString() and