Adjusted sources and sanitizer of UnsafeCertTrust taint tracking config

Author: atorralba

java/ql/lib/semmle/code/java/security/Encryption.qll

@@ -122,6 +122,30 @@ class SetDefaultHostnameVerifierMethod extends Method {
   }
 }
 
+/** The `beginHandshake` method of the class `javax.net.ssl.SSLEngine`. */
+class BeginHandshakeMethod extends Method {
+  BeginHandshakeMethod() {
+    this.hasName("beginHandshake") and
+    this.getDeclaringType().getASupertype*() instanceof SSLEngine
+  }
+}
+
+/** The `wrap` method of the class `javax.net.ssl.SSLEngine`. */
+class SslWrapMethod extends Method {
+  SslWrapMethod() {
+    this.hasName("wrap") and
+    this.getDeclaringType().getASupertype*() instanceof SSLEngine
+  }
+}
+
+/** The `unwrap` method of the class `javax.net.ssl.SSLEngine`. */
+class SslUnwrapMethod extends Method {
+  SslUnwrapMethod() {
+    this.hasName("unwrap") and
+    this.getDeclaringType().getASupertype*() instanceof SSLEngine
+  }
+}
+
 /** The `getSession` method of the class `javax.net.ssl.SSLSession`.select */
 class GetSslSessionMethod extends Method {
   GetSslSessionMethod() {

java/ql/lib/semmle/code/java/security/UnsafeCertTrust.qll

@@ -8,6 +8,7 @@ private import semmle.code.java.dataflow.DataFlow2
 
 /**
  * The creation of an object that prepares an SSL connection.
+ *
  * This is a source for `SslEndpointIdentificationFlowConfig`.
  */
 class SslConnectionInit extends DataFlow::Node {
@@ -19,12 +20,15 @@ class SslConnectionInit extends DataFlow::Node {
 
 /**
  * A call to a method that establishes an SSL connection.
+ *
  * This is a sink for `SslEndpointIdentificationFlowConfig`.
  */
 class SslConnectionCreation extends DataFlow::Node {
   SslConnectionCreation() {
     exists(MethodAccess ma, Method m |
-      m instanceof GetSslSessionMethod or
+      m instanceof BeginHandshakeMethod or
+      m instanceof SslWrapMethod or
+      m instanceof SslUnwrapMethod or
       m instanceof SocketConnectMethod
     |
       ma.getMethod() = m and
@@ -44,10 +48,16 @@ class SslConnectionCreation extends DataFlow::Node {
 }
 
 /**
- * An SSL object that was assigned a safe `SSLParameters` object and can be considered safe.
+ * An SSL object that correctly verifies hostnames, or doesn't need to (because e.g. it's a server).
+ *
  * This is a sanitizer for `SslEndpointIdentificationFlowConfig`.
  */
-class SslConnectionWithSafeSslParameters extends DataFlow::Node {
+abstract class SslUnsafeCertTrustSanitizer extends DataFlow::Node { }
+
+/**
+ * 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 |
       config.hasFlowTo(safe) and
@@ -56,6 +66,21 @@ class SslConnectionWithSafeSslParameters extends DataFlow::Node {
   }
 }
 
+/**
+ * An `SSLEngine` set in server mode.
+ */
+private class SslEngineServerMode extends SslUnsafeCertTrustSanitizer {
+  SslEngineServerMode() {
+    exists(MethodAccess ma, Method m |
+      m.hasName("setUseClientMode") and
+      m.getDeclaringType().getASupertype*() instanceof SSLEngine and
+      ma.getMethod() = m and
+      ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = false and
+      this = DataFlow::exprNode(ma.getQualifier())
+    )
+  }
+}
+
 /**
  * Holds if the return value of `createSocket` is cast to `SSLSocket`
  * or the qualifier of `createSocket` is an instance of `SSLSocketFactory`.

java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql

@@ -23,7 +23,7 @@ class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation }
 
   override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof SslConnectionWithSafeSslParameters
+    sanitizer instanceof SslUnsafeCertTrustSanitizer
   }
 }