From ac8dec740a38a5c2502337652f0c109d37d06fd9 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Mon, 20 Mar 2023 07:35:20 -0400
Subject: [PATCH] Refactor UnsafeCertTrustQuery

---
 .../java/security/UnsafeCertTrustQuery.qll    | 28 +++++++++++++------
 .../Security/CWE/CWE-273/UnsafeCertTrust.ql   |  4 +--
 .../security/CWE-273/UnsafeCertTrustTest.ql   |  4 +--
 3 files changed, 22 insertions(+), 14 deletions(-)

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<SslEndpointIdentificationFlowConfig>;
+
 /**
  * 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<SafeSslParametersFlowConfig>;
+
 /**
  * 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