Move from experimental

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java

@@ -0,0 +1,52 @@
+public static void main(String[] args) throws Exception {
+    {
+        class InsecureTrustManager implements X509TrustManager {
+            @Override
+            public X509Certificate[] getAcceptedIssuers() {
+                return null;
+            }
+
+            @Override
+            public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
+                // BAD: Does not verify the certificate chain, allowing any certificate.
+            }
+
+            @Override
+            public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
+
+            }
+        }
+        SSLContext context = SSLContext.getInstance("TLS");
+        TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
+        context.init(null, trustManager, null);
+    }
+    {
+        SSLContext context = SSLContext.getInstance("TLS");
+        File certificateFile = new File("path/to/self-signed-certificate");
+        // Create a `KeyStore` with default type
+        KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
+        // `keyStore` is initially empty
+        keyStore.load(null, null);
+        X509Certificate generatedCertificate;
+        try (InputStream cert = new FileInputStream(certificateFile)) {
+            generatedCertificate = (X509Certificate) CertificateFactory.getInstance("X509")
+                    .generateCertificate(cert);
+        }
+        // Add the self-signed certificate to the key store
+        keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate);
+        // Get default `TrustManagerFactory`
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+        // Use it with our key store that trusts our self-signed certificate
+        tmf.init(keyStore);
+        TrustManager[] trustManagers = tmf.getTrustManagers();
+        context.init(null, trustManagers, null);
+        // GOOD, we are not using a custom `TrustManager` but instead have
+        // added the self-signed certificate we want to trust to the key
+        // store. Note, the `trustManagers` will **only** trust this one
+        // certificate.
+        
+        URL url = new URL("https://self-signed.badssl.com/");
+        HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
+        conn.setSSLSocketFactory(context.getSocketFactory());
+    }
+}

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp

@@ -0,0 +1,47 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+If the <code>checkServerTrusted</code> method of a <code>TrustManager</code> never throws a <code>CertificateException</code> it trusts every certificate.
+This allows an attacker to perform a machine-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives.
+</p>
+
+<p>
+An attack might look like this:
+</p>
+
+<ol>
+  <li>The vulnerable program connects to <code>https://example.com</code>.</li>
+  <li>The attacker intercepts this connection and presents a valid, self-signed certificate for <code>https://example.com</code>.</li>
+  <li>The vulnerable program calls the <code>checkServerTrusted</code> method to check whether it should trust the certificate.</li>
+  <li>The <code>checkServerTrusted</code> method of your <code>TrustManager</code> does not throw a <code>CertificateException</code>.</li>
+  <li>The vulnerable program accepts the certificate and proceeds with the connection since your <code>TrustManager</code> implicitly trusted it by not throwing an exception.</li>
+  <li>The attacker can now read the data your program sends to <code>https://example.com</code> and/or alter its replies while the program thinks the connection is secure.</li>
+</ol>
+</overview>
+
+<recommendation>
+<p>
+Do not use a custom <code>TrustManager</code> that trusts any certificate.
+If you have to use a self-signed certificate, don't trust every certificate, but instead only trust this specific certificate.
+See below for an example of how to do this.
+</p>
+
+</recommendation>
+
+<example>
+<p>
+In the first (bad) example, the <code>TrustManager</code> never throws a <code>CertificateException</code> and therefore implicitly trusts any certificate.
+This allows an attacker to perform a machine-in-the-middle attack.
+In the second (good) example, the self-signed certificate that should be trusted
+is loaded into a <code>KeyStore</code>. This explicitly defines the certificate as trusted and there is no need to create a custom <code>TrustManager</code>.
+</p>
+<sample src="InsecureTrustManager.java" />
+</example>
+
+<references>
+<li>Android Develoers:<a href="https://developer.android.com/training/articles/security-ssl">Security with HTTPS and SSL</a>.</li>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql

@@ -0,0 +1,117 @@
+/**
+ * @name `TrustManager` that accepts all certificates
+ * @description Trusting all certificates allows an attacker to perform a machine-in-the-middle attack.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/insecure-trustmanager
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+
+import java
+import semmle.code.java.controlflow.Guards
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.Encryption
+import semmle.code.java.security.SecurityFlag
+import DataFlow::PathGraph
+
+/**
+ * An insecure `X509TrustManager`.
+ * An `X509TrustManager` is considered insecure if it never throws a `CertificateException`
+ * and therefore implicitly trusts any certificate as valid.
+ */
+class InsecureX509TrustManager extends RefType {
+  InsecureX509TrustManager() {
+    this.getASupertype*() instanceof X509TrustManager and
+    exists(Method m |
+      m.getDeclaringType() = this and
+      m.hasName("checkServerTrusted") and
+      not mayThrowCertificateException(m)
+    )
+  }
+}
+
+/** The `java.security.cert.CertificateException` class. */
+private class CertificateException extends RefType {
+  CertificateException() { this.hasQualifiedName("java.security.cert", "CertificateException") }
+}
+
+/**
+ * Holds if:
+ * - `m` may `throw` a `CertificateException`, or
+ * - `m` calls another method that may throw, or
+ * - `m` calls a method declared to throw a `CertificateException`, but for which no source is available
+ */
+private predicate mayThrowCertificateException(Method m) {
+  exists(ThrowStmt throwStmt |
+    throwStmt.getThrownExceptionType().getASupertype*() instanceof CertificateException
+  |
+    throwStmt.getEnclosingCallable() = m
+  )
+  or
+  exists(Method otherMethod | m.polyCalls(otherMethod) |
+    mayThrowCertificateException(otherMethod)
+    or
+    not otherMethod.fromSource() and
+    otherMethod.getAnException().getType().getASupertype*() instanceof CertificateException
+  )
+}
+
+/**
+ * A configuration to model the flow of an `InsecureX509TrustManager` to an `SSLContext.init` call.
+ */
+class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
+  InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof InsecureX509TrustManager
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma, Method m |
+      m.hasName("init") and
+      m.getDeclaringType() instanceof SSLContext and
+      ma.getMethod() = m
+    |
+      ma.getArgument(1) = sink.asExpr()
+    )
+  }
+}
+
+/**
+ * Flags suggesting a deliberately insecure `TrustManager` usage.
+ */
+private class InsecureTrustManagerFlag extends FlagKind {
+  InsecureTrustManagerFlag() { this = "InsecureTrustManagerFlag" }
+
+  bindingset[result]
+  override string getAFlagName() {
+    result
+        .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and
+    result != "equalsIgnoreCase"
+  }
+}
+
+/** Gets a guard that represents a (likely) flag controlling an insecure `TrustManager` use. */
+private Guard getAnInsecureTrustManagerFlagGuard() {
+  result = any(InsecureTrustManagerFlag flag).getAFlag().asExpr()
+}
+
+/** Holds if `node` is guarded by a flag that suggests an intentionally insecure use. */
+private predicate isNodeGuardedByFlag(DataFlow::Node node) {
+  exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) |
+    g = getASecurityFeatureFlagGuard() or g = getAnInsecureTrustManagerFlagGuard()
+  )
+}
+
+from
+  DataFlow::PathNode source, DataFlow::PathNode sink, InsecureTrustManagerConfiguration cfg,
+  RefType trustManager
+where
+  cfg.hasFlowPath(source, sink) and
+  not isNodeGuardedByFlag(sink.getNode()) and
+  trustManager = source.getNode().asExpr().(ClassInstanceExpr).getConstructedType()
+select sink, source, sink, "$@ that is defined $@ and trusts any certificate, is used here.",
+  source, "This trustmanager", trustManager, "here"