From 87554a78d4f0fa49ef4a7c7acaddb5237e9f2c66 Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 23 Dec 2020 15:29:13 +0100 Subject: [PATCH 01/28] Java: Add insecure trust manager query. --- .../CWE/CWE-295/InsecureTrustManager.java | 50 +++++ .../CWE/CWE-295/InsecureTrustManager.qhelp | 43 +++++ .../CWE/CWE-295/InsecureTrustManager.ql | 180 ++++++++++++++++++ .../CWE-295/InsecureTrustManager.expected | 10 + .../CWE-295/InsecureTrustManager.qlref | 1 + .../CWE-295/InsecureTrustManagerTest.java | 95 +++++++++ 6 files changed, 379 insertions(+) create mode 100644 java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java create mode 100644 java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp create mode 100644 java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql create mode 100644 java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected create mode 100644 java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref create mode 100644 java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java new file mode 100644 index 000000000000..1024af55ea33 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java @@ -0,0 +1,50 @@ +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()); + // This causes `keyStore` to be 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 modified 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()); + } +} diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp new file mode 100644 index 000000000000..31ca3ae34551 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp @@ -0,0 +1,43 @@ + + + +

+If the checkServerTrusted method of a TrustManager never throws a CertificateException it trusts every certificate. +This allows an attacker to perform a Man-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives. + +An attack would look like this: +1. The program connects to https://example.com. +2. The attacker intercepts this connection and presents a valid, self-signed certificate for https://example.com. +3. Java calls the checkServerTrusted method to check whether it should trust the certificate. +4. The checkServerTrusted method of your TrustManager does not throw a CertificateException. +5. Java proceeds with the connection since your TrustManager implicitly trusted it by not throwing an exception. +6. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure. +

+ + + +

+Do not use a custom TrustManager 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. +

+ + + + +

+In the first (bad) example, the TrustManager never throws a CertificateException thereby trusting any certificate. +This allows an attacker to perform a man-in-the-middle attack. +In the second (good) example, no custom TrustManager is used. Instead, the self-signed certificate that should be trusted +is explicitly trusted by loading it into a KeyStore. +

+ + + + +
  • Android Security Guide for TLS/HTTPS.
    • +
  • OWASP: CWE-295.
    • +     +     diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql new file mode 100644 index 000000000000..6685eec66d56 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -0,0 +1,180 @@ +/** + * @name Everything trusting `TrustManager` + * @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.dataflow.TaintTracking2 +import semmle.code.java.security.Encryption +import DataFlow::PathGraph + +/** + * Models an insecure `X509TrustManager`. + * An `X509TrustManager` is considered insecure if it never throws a `CertificatException` thereby accepting any certificate as valid. + */ +class InsecureX509TrustManager extends RefType { + InsecureX509TrustManager() { + 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 CertificatException extends RefType { + CertificatException() { hasQualifiedName("java.security.cert", "CertificateException") } +} + +/** + *Holds if: + * - `m` may `throw` an `CertificatException` + * - `m` calls another method that may throw + * - `m` calls a method that declares to throw an `CertificatExceptio`, but for which no source is available + */ +private predicate mayThrowCertificateException(Method m) { + exists(Stmt stmt | m.getBody().getAChild*() = stmt | + stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificatException + ) + or + exists(Method otherMethod | m.polyCalls(otherMethod) | + mayThrowCertificateException(otherMethod) + or + not otherMethod.fromSource() and + otherMethod.getAnException().getType().getASupertype*() instanceof CertificatException + ) +} + +/** + * A configuration to model the flow of a `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() + ) + } + + override predicate isSanitizer(DataFlow::Node barrier) { + // ignore nodes that are in functions that intentionally trust all certificates + barrier + .getEnclosingCallable() + .getName() + /* + * Regex: (_)* : + * some methods have underscores. + * Regex: (no|ignore|disable)(strictssl|ssl|verify|verification) + * noStrictSSL ignoreSsl + * Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned) + * acceptAll trustAll ignoreAll setTrustAnyHttps + * Regex: (use|do|enable)insecure + * useInsecureSSL + * Regex: (set|do|use)?no.*(check|validation|verify|verification) + * setNoCertificateCheck + * Regex: disable + * disableChecks + */ + + .regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" + + "|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" + + "|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$") + } +} + +bindingset[result] +private string getAFlagName() { + result + .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") +} + +/** + * A flag has to either be of type `String`, `boolean` or `Boolean`. + */ +private class FlagType extends Type { + FlagType() { + this instanceof TypeString + or + this instanceof BooleanType + } +} + +private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) { + ma.getMethod().hasName("equalsIgnoreCase") and + ma.getMethod().getDeclaringType() instanceof TypeString +} + +/** Holds if `source` should is considered a flag. */ +private predicate isFlag(DataFlow::Node source) { + exists(VarAccess v | v.getVariable().getName() = getAFlagName() | + source.asExpr() = v and v.getType() instanceof FlagType + ) + or + exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s) + or + exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | + source.asExpr() = ma and + ma.getType() instanceof FlagType and + not isEqualsIgnoreCaseMethodAccess(ma) + ) +} + +/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */ +private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + DataFlow::localFlowStep(node1, node2) + or + exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) | + ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() + ) + or + exists(MethodAccess ma | + ma.getMethod().hasName("parseBoolean") and + ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean") + | + ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() + ) +} + +/** Gets a guard that depends on a flag. */ +private Guard getAGuard() { + exists(DataFlow::Node source, DataFlow::Node sink | + isFlag(source) and + flagFlowStep*(source, sink) and + sink.asExpr() = result + ) +} + +/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */ +private predicate isNodeGuardedByFlag(DataFlow::Node node) { + exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | g = getAGuard()) +} + +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" diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected new file mode 100644 index 000000000000..de68af38077e --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected @@ -0,0 +1,10 @@ +edges +| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | +| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:49:24:49:35 | trustManager | +nodes +| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:49:24:49:35 | trustManager | semmle.label | trustManager | +#select +| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:20:23:20:42 | InsecureTrustManager | here | diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref new file mode 100644 index 000000000000..9bb2ecf04e83 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-295/InsecureTrustManager.ql diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java new file mode 100644 index 000000000000..3858aa2f63bc --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java @@ -0,0 +1,95 @@ +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStream; +import java.net.URL; +import java.security.KeyStore; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; + +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509TrustManager; + +public class InsecureTrustManagerTest { + + private static final boolean TRUST_ALL = true; + + private static 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 { + + } + } + + public static void main(String[] args) throws Exception { + { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. + } + + { + if (TRUST_ALL) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the + // certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + { + disableTrustManager(); + } + { + 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()); + // This causes `keyStore` to be 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 modified 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()); + } + } + + private static void disableTrustManager() { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the + // certificate + // chain, allowing any certificate. BUT it is the method name suggests that this + // is intentional. + } +} \ No newline at end of file From 1b96d0ac543aa1589b034fc1fee1ab0cab6b58bd Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 23 Dec 2020 16:39:19 +0100 Subject: [PATCH 02/28] Java: Remove overlapping code --- .../Security/CWE/CWE-273/UnsafeCertTrust.java | 40 ----------- .../CWE/CWE-273/UnsafeCertTrust.qhelp | 12 ++-- .../Security/CWE/CWE-273/UnsafeCertTrust.ql | 47 +------------ .../security/CWE-273/UnsafeCertTrustTest.java | 66 ------------------- 4 files changed, 5 insertions(+), 160 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java index b3536fa7d1de..e94491eb22f0 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java +++ b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java @@ -1,45 +1,5 @@ public static void main(String[] args) { - { - X509TrustManager trustAllCertManager = new X509TrustManager() { - @Override - public void checkClientTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - } - - @Override - public void checkServerTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - // BAD: trust any server cert - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; //BAD: doesn't check cert issuer - } - }; - } - - { - X509TrustManager trustCertManager = new X509TrustManager() { - @Override - public void checkClientTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - } - - @Override - public void checkServerTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - pkixTrustManager.checkServerTrusted(chain, authType); //GOOD: validate the server cert - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; //GOOD: Validate the cert issuer - } - }; - } - { SSLContext sslContext = SSLContext.getInstance("TLS"); SSLEngine sslEngine = sslContext.createSSLEngine(); diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp index 9a6bdb82cbac..ae8d76b1bb1f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp @@ -4,10 +4,9 @@ -

    Java offers two mechanisms for SSL authentication - trust manager and hostname verifier (checked by the java/insecure-hostname-verifier query). Trust manager validates the peer's certificate chain while hostname verification establishes that the hostname in the URL matches the hostname in the server's identification.

    -

    And when SSLSocket or SSLEngine is created without a valid parameter of setEndpointIdentificationAlgorithm, hostname verification is disabled by default.

    +

    When SSLSocket or SSLEngine is created without a valid parameter of setEndpointIdentificationAlgorithm, hostname verification is disabled by default.

    Unsafe implementation of the interface X509TrustManager and SSLSocket/SSLEngine ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.

    -

    This query checks whether trust manager is set to trust all certificates or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.

    +

    This query checks whether setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.

     @@ -15,8 +14,8 @@ -

    The following two examples show two ways of configuring X509 trust cert manager. In the 'BAD' case, -no validation is performed thus any certificate is trusted. In the 'GOOD' case, the proper validation is performed.

    +

    The following two examples show two ways of configuring SSLSocket/SSLEngine. In the 'BAD' case, +setEndpointIdentificationAlgorithm is not called, thus no hostname verification takes place. In the 'GOOD' case, setEndpointIdentificationAlgorithm is called.

     @@ -25,9 +24,6 @@ no validation is performed thus any certificate is trusted. In the 'GOOD' case, CWE-273
  • -How to fix apps containing an unsafe implementation of TrustManager -
    • -
  • Testing Endpoint Identify Verification (MSTG-NETWORK-3)
  • diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql index 9efdcbf4c6e0..a18b35ae38f4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql @@ -1,7 +1,6 @@ /** * @name Unsafe certificate trust - * @description Unsafe implementation of the interface X509TrustManager and - * SSLSocket/SSLEngine ignores all SSL certificate validation + * @description SSLSocket/SSLEngine ignores all SSL certificate validation * errors when establishing an HTTPS connection, thereby making * the app vulnerable to man-in-the-middle attacks. * @kind problem @@ -15,49 +14,6 @@ import java import semmle.code.java.security.Encryption -/** - * X509TrustManager class that blindly trusts all certificates in server SSL authentication - */ -class X509TrustAllManager extends RefType { - X509TrustAllManager() { - this.getASupertype*() instanceof X509TrustManager and - exists(Method m1 | - m1.getDeclaringType() = this and - m1.hasName("checkServerTrusted") and - m1.getBody().getNumStmt() = 0 - ) and - exists(Method m2, ReturnStmt rt2 | - m2.getDeclaringType() = this and - m2.hasName("getAcceptedIssuers") and - rt2.getEnclosingCallable() = m2 and - rt2.getResult() instanceof NullLiteral - ) - } -} - -/** - * The init method of SSLContext with the trust all manager, which is sslContext.init(..., serverTMs, ...) - */ -class X509TrustAllManagerInit extends MethodAccess { - X509TrustAllManagerInit() { - this.getMethod().hasName("init") and - this.getMethod().getDeclaringType() instanceof SSLContext and //init method of SSLContext - ( - exists(ArrayInit ai | - this.getArgument(1).(ArrayCreationExpr).getInit() = ai and - ai.getInit(0).(VarAccess).getVariable().getInitializer().getType().(Class).getASupertype*() - instanceof X509TrustAllManager //Scenario of context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null); - ) - or - exists(Variable v, ArrayInit ai | - this.getArgument(1).(VarAccess).getVariable() = v and - ai.getParent() = v.getAnAssignedValue() and - ai.getInit(0).getType().(Class).getASupertype*() instanceof X509TrustAllManager //Scenario of context.init(null, serverTMs, null); - ) - ) - } -} - class SSLEngine extends RefType { SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") } } @@ -208,7 +164,6 @@ class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess { from MethodAccess aa where - aa instanceof X509TrustAllManagerInit or aa instanceof SSLEndpointIdentificationNotSet or aa instanceof RabbitMQEnableHostnameVerificationNotSet select aa, "Unsafe configuration of trusted certificates" diff --git a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java index 1e8ecbbc20d1..cb8b472eb8fd 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java +++ b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java @@ -18,72 +18,6 @@ public class UnsafeCertTrustTest { - /** - * Test the implementation of trusting all server certs as a variable - */ - public SSLSocketFactory testTrustAllCertManager() { - try { - final SSLContext context = SSLContext.getInstance("TLS"); - context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null); - final SSLSocketFactory socketFactory = context.getSocketFactory(); - return socketFactory; - } catch (final Exception x) { - throw new RuntimeException(x); - } - } - - /** - * Test the implementation of trusting all server certs as an anonymous class - */ - public SSLSocketFactory testTrustAllCertManagerOfVariable() { - try { - SSLContext context = SSLContext.getInstance("TLS"); - TrustManager[] serverTMs = new TrustManager[] { new X509TrustAllManager() }; - context.init(null, serverTMs, null); - - final SSLSocketFactory socketFactory = context.getSocketFactory(); - return socketFactory; - } catch (final Exception x) { - throw new RuntimeException(x); - } - } - - private static final X509TrustManager TRUST_ALL_CERTIFICATES = new X509TrustManager() { - @Override - public void checkClientTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - } - - @Override - public void checkServerTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - // Noncompliant - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; // Noncompliant - } - }; - - private class X509TrustAllManager implements X509TrustManager { - @Override - public void checkClientTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - } - - @Override - public void checkServerTrusted(final X509Certificate[] chain, final String authType) - throws CertificateException { - // Noncompliant - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; // Noncompliant - } - }; - /** * Test the endpoint identification of SSL engine is set to null */ From 592fd1e8ca9b7d1b37a35e3f2bfd51264f0b97be Mon Sep 17 00:00:00 2001 From: intrigus Date: Sat, 16 Jan 2021 14:07:20 +0100 Subject: [PATCH 03/28] Java: Accept test changes --- .../query-tests/security/CWE-273/UnsafeCertTrust.expected | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected index 5d2da21289e5..f26706a56d2c 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected @@ -1,5 +1,3 @@ -| UnsafeCertTrustTest.java:27:4:27:74 | init(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:42:4:42:38 | init(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:92:25:92:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:103:25:103:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:112:34:112:83 | createSocket(...) | Unsafe configuration of trusted certificates | +| UnsafeCertTrustTest.java:26:25:26:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | +| UnsafeCertTrustTest.java:37:25:37:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | +| UnsafeCertTrustTest.java:46:34:46:83 | createSocket(...) | Unsafe configuration of trusted certificates | From f52e438f3edf67c74336fff8a84b3d2341b631c9 Mon Sep 17 00:00:00 2001 From: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com> Date: Wed, 27 Jan 2021 21:48:32 +0100 Subject: [PATCH 04/28] Java: Apply suggestions from code review Co-authored-by: Chris Smowton --- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java | 4 ++-- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java index 1024af55ea33..fd844183fc0a 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java @@ -25,7 +25,7 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) throws File certificateFile = new File("path/to/self-signed-certificate"); // Create a `KeyStore` with default type KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - // This causes `keyStore` to be empty + // `keyStore` is initially empty keyStore.load(null, null); X509Certificate generatedCertificate; try (InputStream cert = new FileInputStream(certificateFile)) { @@ -36,7 +36,7 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) throws keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate); // Get default `TrustManagerFactory` TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - // Use it with our modified key store that trusts our self-signed certificate + // 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 diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 6685eec66d56..07b846a212ee 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -1,5 +1,5 @@ /** - * @name Everything trusting `TrustManager` + * @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 From 030c2869022d5686d01fd27730fdbd9c80115e5b Mon Sep 17 00:00:00 2001 From: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com> Date: Wed, 27 Jan 2021 21:49:34 +0100 Subject: [PATCH 05/28] Java: Use machine-in-the-middle consistently --- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp index 31ca3ae34551..2aec2e16b7a3 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp @@ -5,7 +5,7 @@

    If the checkServerTrusted method of a TrustManager never throws a CertificateException it trusts every certificate. -This allows an attacker to perform a Man-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives. +This allows an attacker to perform a machine-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives. An attack would look like this: 1. The program connects to https://example.com. @@ -29,7 +29,7 @@ See below for an example of how to do this.

    In the first (bad) example, the TrustManager never throws a CertificateException thereby trusting any certificate. -This allows an attacker to perform a man-in-the-middle attack. +This allows an attacker to perform a machine-in-the-middle attack. In the second (good) example, no custom TrustManager is used. Instead, the self-signed certificate that should be trusted is explicitly trusted by loading it into a KeyStore.

    From d37d922e8f6a03e8f7947eb971821af985803be8 Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 27 Jan 2021 22:07:45 +0100 Subject: [PATCH 06/28] Java: Fix Typos --- .../Security/CWE/CWE-295/InsecureTrustManager.ql | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 07b846a212ee..799d2a76b325 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -19,7 +19,7 @@ import DataFlow::PathGraph /** * Models an insecure `X509TrustManager`. - * An `X509TrustManager` is considered insecure if it never throws a `CertificatException` thereby accepting any certificate as valid. + * An `X509TrustManager` is considered insecure if it never throws a `CertificateException` thereby accepting any certificate as valid. */ class InsecureX509TrustManager extends RefType { InsecureX509TrustManager() { @@ -33,26 +33,26 @@ class InsecureX509TrustManager extends RefType { } /** The `java.security.cert.CertificateException` class. */ -private class CertificatException extends RefType { - CertificatException() { hasQualifiedName("java.security.cert", "CertificateException") } +private class CertificateException extends RefType { + CertificateException() { hasQualifiedName("java.security.cert", "CertificateException") } } /** - *Holds if: - * - `m` may `throw` an `CertificatException` + * Holds if: + * - `m` may `throw` a `CertificateException` * - `m` calls another method that may throw - * - `m` calls a method that declares to throw an `CertificatExceptio`, but for which no source is available + * - `m` calls a method declared to throw a `CertificateException`, but for which no source is available */ private predicate mayThrowCertificateException(Method m) { exists(Stmt stmt | m.getBody().getAChild*() = stmt | - stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificatException + stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificateException ) or exists(Method otherMethod | m.polyCalls(otherMethod) | mayThrowCertificateException(otherMethod) or not otherMethod.fromSource() and - otherMethod.getAnException().getType().getASupertype*() instanceof CertificatException + otherMethod.getAnException().getType().getASupertype*() instanceof CertificateException ) } From 8a7f6b72e96d3d26ecd36e932ffc4aec35d4e2ee Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 27 Jan 2021 22:10:48 +0100 Subject: [PATCH 07/28] Java: Apply suggestions for QHelp --- .../CWE/CWE-295/InsecureTrustManager.qhelp | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp index 2aec2e16b7a3..5f7b4ec39ac1 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp @@ -6,14 +6,20 @@

    If the checkServerTrusted method of a TrustManager never throws a CertificateException 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. +

    + +

    +An attack might look like this: +

    -An attack would look like this: -1. The program connects to https://example.com. -2. The attacker intercepts this connection and presents a valid, self-signed certificate for https://example.com. -3. Java calls the checkServerTrusted method to check whether it should trust the certificate. -4. The checkServerTrusted method of your TrustManager does not throw a CertificateException. -5. Java proceeds with the connection since your TrustManager implicitly trusted it by not throwing an exception. -6. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure. +
      +
    1. The vulnerable program connects to https://example.com. +
    2. The attacker intercepts this connection and presents a valid, self-signed certificate for https://example.com. +
    3. The vulnerable program calls the checkServerTrusted method to check whether it should trust the certificate. +
    4. The checkServerTrusted method of your TrustManager does not throw a CertificateException. +
    5. The vulnerable program accepts the certificate and proceeds with the connection since your TrustManager implicitly trusted it by not throwing an exception. +
    6. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure. +

     From e4775e0faee885a75f4c3143d305c58abf13767d Mon Sep 17 00:00:00 2001 From: intrigus Date: Sat, 10 Apr 2021 14:48:04 +0200 Subject: [PATCH 08/28] Java: Remove "intention-guessing" sanitizer & simplify. This removes the sanitizer part that classified some results as FP if the results were in methods with certain names, like `disableVerification()`. I now think that it's a bad idea to filter based on the method name. The custom flow steps in `flagFlowStep` are now listed explicitly. Simplified check whether a method throws an exception. --- .../CWE/CWE-295/InsecureTrustManager.ql | 37 ++++--------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 799d2a76b325..f122b0f5cf31 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -44,9 +44,8 @@ private class CertificateException extends RefType { * - `m` calls a method declared to throw a `CertificateException`, but for which no source is available */ private predicate mayThrowCertificateException(Method m) { - exists(Stmt stmt | m.getBody().getAChild*() = stmt | - stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificateException - ) + m.getBody().getAChild*().(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof + CertificateException or exists(Method otherMethod | m.polyCalls(otherMethod) | mayThrowCertificateException(otherMethod) @@ -75,31 +74,6 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration { ma.getArgument(1) = sink.asExpr() ) } - - override predicate isSanitizer(DataFlow::Node barrier) { - // ignore nodes that are in functions that intentionally trust all certificates - barrier - .getEnclosingCallable() - .getName() - /* - * Regex: (_)* : - * some methods have underscores. - * Regex: (no|ignore|disable)(strictssl|ssl|verify|verification) - * noStrictSSL ignoreSsl - * Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned) - * acceptAll trustAll ignoreAll setTrustAnyHttps - * Regex: (use|do|enable)insecure - * useInsecureSSL - * Regex: (set|do|use)?no.*(check|validation|verify|verification) - * setNoCertificateCheck - * Regex: disable - * disableChecks - */ - - .regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" + - "|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" + - "|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$") - } } bindingset[result] @@ -139,7 +113,12 @@ private predicate isFlag(DataFlow::Node source) { ) } -/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */ +/** + * Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps: + * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. + * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. + * The return value of such a method is then tainted. + */ private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { DataFlow::localFlowStep(node1, node2) or From 6d09db6fd6966eef586bd7bce789aa16ba0a8510 Mon Sep 17 00:00:00 2001 From: intrigus Date: Sat, 10 Apr 2021 14:53:02 +0200 Subject: [PATCH 09/28] Java: Explicitly list custom flow steps. --- .../src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql index 1828b924752c..5fce4a588ea7 100644 --- a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql +++ b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql @@ -123,7 +123,12 @@ private predicate isFlag(DataFlow::Node source) { ) } -/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */ +/** + * Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps: + * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. + * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. + * The return value of such a method is then tainted. + */ private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { DataFlow::localFlowStep(node1, node2) or From 7023793af485483721cb3ff52a9d7da96972b682 Mon Sep 17 00:00:00 2001 From: intrigus Date: Mon, 12 Apr 2021 15:16:02 +0200 Subject: [PATCH 10/28] Java: Fix compilation errors in test. --- .../security/CWE-295/InsecureTrustManagerTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java index 3858aa2f63bc..a43498099b85 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java @@ -2,7 +2,9 @@ import java.io.FileInputStream; import java.io.InputStream; import java.net.URL; +import java.security.KeyManagementException; import java.security.KeyStore; +import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -84,7 +86,7 @@ public static void main(String[] args) throws Exception { } } - private static void disableTrustManager() { + private static void disableTrustManager() throws NoSuchAlgorithmException, KeyManagementException { SSLContext context = SSLContext.getInstance("TLS"); TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the From 484533c659a317c46b9b380c3f66cc4f6058607d Mon Sep 17 00:00:00 2001 From: intrigus Date: Mon, 12 Apr 2021 15:19:18 +0200 Subject: [PATCH 11/28] Java: Flag "intentionally" unsafe methods in tests. Previously intentionally unsafe methods such as `disableCertificate` would be ignored by this query. But now they will also be flagged as it is hard to guess intentions... Adjust the tests to account for this change. --- .../CWE-295/InsecureTrustManager.expected | 18 +++++++++++------- .../CWE-295/InsecureTrustManagerTest.java | 6 +++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected index de68af38077e..b7d3703c32f0 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected @@ -1,10 +1,14 @@ edges -| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | -| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:49:24:49:35 | trustManager | +| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager | +| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:51:24:51:35 | trustManager | +| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager | nodes -| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | -| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | semmle.label | trustManager | -| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | -| InsecureTrustManagerTest.java:49:24:49:35 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:51:24:51:35 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | semmle.label | trustManager | #select -| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:20:23:20:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here | diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java index a43498099b85..db655ac85415 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java @@ -89,9 +89,9 @@ public static void main(String[] args) throws Exception { private static void disableTrustManager() throws NoSuchAlgorithmException, KeyManagementException { SSLContext context = SSLContext.getInstance("TLS"); TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; - context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the // certificate - // chain, allowing any certificate. BUT it is the method name suggests that this - // is intentional. + // chain, allowing any certificate. The method name suggests that this may be + // intentional, but we flag it anyway. } } \ No newline at end of file From 6413af4fbe40a1418895742015401207478baa76 Mon Sep 17 00:00:00 2001 From: intrigus Date: Tue, 13 Apr 2021 20:26:35 +0200 Subject: [PATCH 12/28] Java: Expand tests. --- .../CWE-295/InsecureTrustManagerTest.java | 399 ++++++++++++++++-- 1 file changed, 361 insertions(+), 38 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java index db655ac85415..5b314d464d48 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java @@ -1,9 +1,13 @@ import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; import java.io.InputStream; +import java.net.MalformedURLException; import java.net.URL; import java.security.KeyManagementException; import java.security.KeyStore; +import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -18,6 +22,15 @@ public class InsecureTrustManagerTest { private static final boolean TRUST_ALL = true; + private static final boolean SOME_NAME_THAT_IS_NOT_A_FLAG_NAME = true; + + private static boolean isDisableTrust() { + return true; + } + + private static boolean is42TheAnswerForEverything() { + return true; + } private static class InsecureTrustManager implements X509TrustManager { @Override @@ -37,55 +50,365 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) throws } public static void main(String[] args) throws Exception { - { + directInsecureTrustManagerCall(); + + namedVariableFlagDirectInsecureTrustManagerCall(); + noNamedVariableFlagDirectInsecureTrustManagerCall(); + namedVariableFlagIndirectInsecureTrustManagerCall(); + noNamedVariableFlagIndirectInsecureTrustManagerCall(); + + stringLiteralFlagDirectInsecureTrustManagerCall(); + noStringLiteralFlagDirectInsecureTrustManagerCall(); + stringLiteralFlagIndirectInsecureTrustManagerCall(); + noStringLiteralFlagIndirectInsecureTrustManagerCall(); + + methodAccessFlagDirectInsecureTrustManagerCall(); + noMethodAccessFlagDirectInsecureTrustManagerCall(); + methodAccessFlagIndirectInsecureTrustManagerCall(); + noMethodAccessFlagIndirectInsecureTrustManagerCall(); + + isEqualsIgnoreCaseDirectInsecureTrustManagerCall(); + noIsEqualsIgnoreCaseDirectInsecureTrustManagerCall(); + isEqualsIgnoreCaseIndirectInsecureTrustManagerCall(); + noIsEqualsIgnoreCaseIndirectInsecureTrustManagerCall(); + + namedVariableFlagNOTGuardingDirectInsecureTrustManagerCall(); + noNamedVariableFlagNOTGuardingDirectInsecureTrustManagerCall(); + + stringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall(); + noStringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall(); + + methodAccessFlagNOTGuardingDirectInsecureTrustManagerCall(); + noMethodAccessFlagNOTGuardingDirectInsecureTrustManagerCall(); + + isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall(); + noIsEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall(); + + directSecureTrustManagerCall(); + + } + + private static void directSecureTrustManagerCall() throws NoSuchAlgorithmException, KeyStoreException, IOException, + CertificateException, FileNotFoundException, KeyManagementException, MalformedURLException { + 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()); + // This causes `keyStore` to be 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 modified 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()); + } + + private static void directInsecureTrustManagerCall() throws NoSuchAlgorithmException, KeyManagementException { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. + } + + private static void namedVariableFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (TRUST_ALL) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void namedVariableFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (TRUST_ALL) { + disableTrustManager(); // GOOD [But the disableTrustManager method itself is still detected]: Calls a + // method that install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void noNamedVariableFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (SOME_NAME_THAT_IS_NOT_A_FLAG_NAME) { SSLContext context = SSLContext.getInstance("TLS"); TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate - // chain, allowing any certificate. + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void noNamedVariableFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (SOME_NAME_THAT_IS_NOT_A_FLAG_NAME) { + disableTrustManager(); // BAD [This is detected in the disableTrustManager method]: Calls a method that + // install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void stringLiteralFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("TRUST_ALL"))) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void stringLiteralFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("TRUST_ALL"))) { + disableTrustManager(); // GOOD [But the disableTrustManager method itself is still detected]: Calls a + // method that install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void noStringLiteralFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("SOME_NAME_THAT_IS_NOT_A_FLAG_NAME"))) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void noStringLiteralFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("SOME_NAME_THAT_IS_NOT_A_FLAG_NAME"))) { + disableTrustManager(); // BAD [This is detected in the disableTrustManager method]: Calls a method that + // install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void methodAccessFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (isDisableTrust()) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void methodAccessFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (isDisableTrust()) { + disableTrustManager(); // GOOD [But the disableTrustManager method itself is still detected]: Calls a + // method that install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. BUT it is guarded + // by a feature flag. + } + } + + private static void noMethodAccessFlagDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (is42TheAnswerForEverything()) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void noMethodAccessFlagIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (is42TheAnswerForEverything()) { + disableTrustManager(); // BAD [This is detected in the disableTrustManager method]: Calls a method that + // install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. } + } - { - if (TRUST_ALL) { - SSLContext context = SSLContext.getInstance("TLS"); - TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; - context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the - // certificate - // chain, allowing any certificate. BUT it is guarded - // by a feature flag. - } + private static void isEqualsIgnoreCaseDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (schemaFromHttpRequest.equalsIgnoreCase("https")) { + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. } - { - disableTrustManager(); + } + + private static void isEqualsIgnoreCaseIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (schemaFromHttpRequest.equalsIgnoreCase("https")) { + disableTrustManager(); // BAD [This is detected in the disableTrustManager method]: Calls a method that + // install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. } - { + } + + private static void noIsEqualsIgnoreCaseDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (!schemaFromHttpRequest.equalsIgnoreCase("https")) { 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()); - // This causes `keyStore` to be 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 modified 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()); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. } } + private static void noIsEqualsIgnoreCaseIndirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (!schemaFromHttpRequest.equalsIgnoreCase("https")) { + disableTrustManager(); // BAD [This is detected in the disableTrustManager method]: Calls a method that + // install a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag. + } + } + + private static void namedVariableFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (TRUST_ALL) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if. + + } + + private static void noNamedVariableFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (SOME_NAME_THAT_IS_NOT_A_FLAG_NAME) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if and it is NOT a valid flag. + + } + + private static void stringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("TRUST_ALL"))) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if. + + } + + private static void noStringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (Boolean.parseBoolean(System.getProperty("SOME_NAME_THAT_IS_NOT_A_FLAG_NAME"))) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if and it is NOT a valid flag. + + } + + private static void methodAccessFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (isDisableTrust()) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if. + + } + + private static void noMethodAccessFlagNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + if (is42TheAnswerForEverything()) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if and it is NOT a valid flag. + + } + + private static void isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (schemaFromHttpRequest.equalsIgnoreCase("https")) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if and it is NOT a valid flag. + + } + + private static void noIsEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall() + throws NoSuchAlgorithmException, KeyManagementException { + String schemaFromHttpRequest = "HTTPS"; + if (!schemaFromHttpRequest.equalsIgnoreCase("https")) { + System.out.println("Disabling trust!"); + } + + SSLContext context = SSLContext.getInstance("TLS"); + TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; + context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate + // chain, allowing any certificate. It is NOT guarded + // by a feature flag, because it is outside the if and it is NOT a valid flag. + + } + private static void disableTrustManager() throws NoSuchAlgorithmException, KeyManagementException { SSLContext context = SSLContext.getInstance("TLS"); TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() }; From 281e0859d1a48eefab4f950ba063201773037b92 Mon Sep 17 00:00:00 2001 From: intrigus Date: Tue, 13 Apr 2021 20:27:11 +0200 Subject: [PATCH 13/28] Java: Accept test changes. --- .../CWE-295/InsecureTrustManager.expected | 80 ++++++++++++++++--- 1 file changed, 69 insertions(+), 11 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected index b7d3703c32f0..cba1d7ede75c 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected @@ -1,14 +1,72 @@ edges -| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager | -| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:51:24:51:35 | trustManager | -| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager | +| InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:122:22:122:33 | trustManager | +| InsecureTrustManagerTest.java:130:55:130:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:131:23:131:34 | trustManager | +| InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:152:23:152:34 | trustManager | +| InsecureTrustManagerTest.java:172:55:172:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:173:23:173:34 | trustManager | +| InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:194:23:194:34 | trustManager | +| InsecureTrustManagerTest.java:214:55:214:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:215:23:215:34 | trustManager | +| InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:236:23:236:34 | trustManager | +| InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:258:23:258:34 | trustManager | +| InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:281:23:281:34 | trustManager | +| InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:306:22:306:33 | trustManager | +| InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:320:22:320:33 | trustManager | +| InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:334:22:334:33 | trustManager | +| InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:348:22:348:33 | trustManager | +| InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:362:22:362:33 | trustManager | +| InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:376:22:376:33 | trustManager | +| InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:391:22:391:33 | trustManager | +| InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:406:22:406:33 | trustManager | +| InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:415:22:415:33 | trustManager | nodes -| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | -| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | semmle.label | trustManager | -| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | -| InsecureTrustManagerTest.java:51:24:51:35 | trustManager | semmle.label | trustManager | -| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | -| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:122:22:122:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:130:55:130:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:131:23:131:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:152:23:152:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:172:55:172:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:173:23:173:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:194:23:194:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:214:55:214:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:215:23:215:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:236:23:236:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:258:23:258:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:281:23:281:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:306:22:306:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:320:22:320:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:334:22:334:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:348:22:348:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:362:22:362:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:376:22:376:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:391:22:391:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:406:22:406:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | +| InsecureTrustManagerTest.java:415:22:415:33 | trustManager | semmle.label | trustManager | #select -| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here | -| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:122:22:122:33 | trustManager | InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:122:22:122:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:152:23:152:34 | trustManager | InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:152:23:152:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:194:23:194:34 | trustManager | InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:194:23:194:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:236:23:236:34 | trustManager | InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:236:23:236:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:258:23:258:34 | trustManager | InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:258:23:258:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:281:23:281:34 | trustManager | InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:281:23:281:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:306:22:306:33 | trustManager | InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:306:22:306:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:320:22:320:33 | trustManager | InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:320:22:320:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:334:22:334:33 | trustManager | InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:334:22:334:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:348:22:348:33 | trustManager | InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:348:22:348:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:362:22:362:33 | trustManager | InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:362:22:362:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:376:22:376:33 | trustManager | InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:376:22:376:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:391:22:391:33 | trustManager | InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:391:22:391:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:406:22:406:33 | trustManager | InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:406:22:406:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | +| InsecureTrustManagerTest.java:415:22:415:33 | trustManager | InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:415:22:415:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:35:23:35:42 | InsecureTrustManager | here | From 0c1ce741352dc0c31806c4ffe6034488e101813f Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 15 Apr 2021 19:34:14 +0200 Subject: [PATCH 14/28] Java: Switch from tabs to spaces. --- .../CWE/CWE-295/InsecureTrustManager.java | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java index fd844183fc0a..bca408f9f013 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java @@ -1,50 +1,50 @@ public static void main(String[] args) throws Exception { - { - class InsecureTrustManager implements X509TrustManager { - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; - } + { + 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 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 { + @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()); - } + } + } + 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()); + } } From 45cec3df1c10aaa7b5f1f2b88fcc7d7a3ef95aad Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 15 Apr 2021 19:36:11 +0200 Subject: [PATCH 15/28] Java: Use `this` consistently in QL classes. --- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index f122b0f5cf31..eb035bc3e654 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -23,7 +23,7 @@ import DataFlow::PathGraph */ class InsecureX509TrustManager extends RefType { InsecureX509TrustManager() { - getASupertype*() instanceof X509TrustManager and + this.getASupertype*() instanceof X509TrustManager and exists(Method m | m.getDeclaringType() = this and m.hasName("checkServerTrusted") and @@ -34,7 +34,7 @@ class InsecureX509TrustManager extends RefType { /** The `java.security.cert.CertificateException` class. */ private class CertificateException extends RefType { - CertificateException() { hasQualifiedName("java.security.cert", "CertificateException") } + CertificateException() { this.hasQualifiedName("java.security.cert", "CertificateException") } } /** From 4a00670b68a02f2db59ed729503243a8ef71e83c Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 15 Apr 2021 19:40:22 +0200 Subject: [PATCH 16/28] Java: Reduce long comment. --- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index eb035bc3e654..24f2f217dea6 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -19,7 +19,8 @@ import DataFlow::PathGraph /** * Models an insecure `X509TrustManager`. - * An `X509TrustManager` is considered insecure if it never throws a `CertificateException` thereby accepting any certificate as valid. + * An `X509TrustManager` is considered insecure if it never throws a `CertificateException` + * thereby accepting any certificate as valid. */ class InsecureX509TrustManager extends RefType { InsecureX509TrustManager() { From 6f217d37da6581db4869a8f7355e49ac0629cf40 Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 15 Apr 2021 21:20:46 +0200 Subject: [PATCH 17/28] Java: Apply suggestions from review. --- .../Security/CWE/CWE-295/InsecureTrustManager.ql | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 24f2f217dea6..c6120421fd1c 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -80,7 +80,8 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration { bindingset[result] private string getAFlagName() { result - .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") + .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and + result != "equalsIgnoreCase" } /** @@ -94,11 +95,6 @@ private class FlagType extends Type { } } -private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) { - ma.getMethod().hasName("equalsIgnoreCase") and - ma.getMethod().getDeclaringType() instanceof TypeString -} - /** Holds if `source` should is considered a flag. */ private predicate isFlag(DataFlow::Node source) { exists(VarAccess v | v.getVariable().getName() = getAFlagName() | @@ -109,13 +105,13 @@ private predicate isFlag(DataFlow::Node source) { or exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | source.asExpr() = ma and - ma.getType() instanceof FlagType and - not isEqualsIgnoreCaseMethodAccess(ma) + ma.getType() instanceof FlagType ) } /** - * Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps: + * Holds if there is local flow from `node1` to `node2` either due to standard data-flow steps or the + * following custom flow steps: * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. * The return value of such a method is then tainted. From 51fdcf86c89fef72c46b43167671088193720291 Mon Sep 17 00:00:00 2001 From: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com> Date: Wed, 28 Apr 2021 21:14:51 +0200 Subject: [PATCH 18/28] Apply suggestions from code review Co-authored-by: Anders Schack-Mulligen --- .../Security/CWE/CWE-295/InsecureTrustManager.ql | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index c6120421fd1c..afe5a2f1d54a 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -18,7 +18,7 @@ import semmle.code.java.security.Encryption import DataFlow::PathGraph /** - * Models an insecure `X509TrustManager`. + * An insecure `X509TrustManager`. * An `X509TrustManager` is considered insecure if it never throws a `CertificateException` * thereby accepting any certificate as valid. */ @@ -40,13 +40,16 @@ private class CertificateException extends RefType { /** * Holds if: - * - `m` may `throw` a `CertificateException` - * - `m` calls another method that may throw + * - `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) { - m.getBody().getAChild*().(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof - CertificateException + exists(ThrowStmt throwStmt | + throwStmt.getThrownExceptionType().getASupertype*() instanceof CertificateException + | + throwStmt.getEnclosingCallable() = m + ) or exists(Method otherMethod | m.polyCalls(otherMethod) | mayThrowCertificateException(otherMethod) @@ -57,7 +60,7 @@ private predicate mayThrowCertificateException(Method m) { } /** - * A configuration to model the flow of a `InsecureX509TrustManager` to an `SSLContext.init` call. + * A configuration to model the flow of an `InsecureX509TrustManager` to an `SSLContext.init` call. */ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration { InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" } From dc0b06a735d247cfe917cd8ee4f991edd686828b Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 28 Apr 2021 22:03:23 +0200 Subject: [PATCH 19/28] Java: Factor out `SecurityFlag` library. --- .../CWE/CWE-295/InsecureTrustManager.ql | 76 ++++------------- .../CWE/CWE-297/UnsafeHostnameVerification.ql | 79 ++++------------- .../code/java/security/SecurityFlag.qll | 84 +++++++++++++++++++ 3 files changed, 118 insertions(+), 121 deletions(-) create mode 100644 java/ql/src/semmle/code/java/security/SecurityFlag.qll diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index afe5a2f1d54a..866641d3cf1e 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -13,8 +13,8 @@ import java import semmle.code.java.controlflow.Guards import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources -import semmle.code.java.dataflow.TaintTracking2 import semmle.code.java.security.Encryption +import semmle.code.java.security.SecurityFlag import DataFlow::PathGraph /** @@ -80,72 +80,30 @@ class InsecureTrustManagerConfiguration extends TaintTracking::Configuration { } } -bindingset[result] -private string getAFlagName() { - result - .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and - result != "equalsIgnoreCase" -} - /** - * A flag has to either be of type `String`, `boolean` or `Boolean`. + * Flags suggesting a deliberately insecure `TrustManager` usage. */ -private class FlagType extends Type { - FlagType() { - this instanceof TypeString - or - this instanceof BooleanType - } -} - -/** Holds if `source` should is considered a flag. */ -private predicate isFlag(DataFlow::Node source) { - exists(VarAccess v | v.getVariable().getName() = getAFlagName() | - source.asExpr() = v and v.getType() instanceof FlagType - ) - or - exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s) - or - exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | - source.asExpr() = ma and - ma.getType() instanceof FlagType - ) -} +private class InsecureTrustManagerFlag extends FlagKind { + InsecureTrustManagerFlag() { this = "InsecureTrustManagerFlag" } -/** - * Holds if there is local flow from `node1` to `node2` either due to standard data-flow steps or the - * following custom flow steps: - * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. - * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. - * The return value of such a method is then tainted. - */ -private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - DataFlow::localFlowStep(node1, node2) - or - exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) | - ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() - ) - or - exists(MethodAccess ma | - ma.getMethod().hasName("parseBoolean") and - ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean") - | - ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() - ) + bindingset[result] + override string getAFlagName() { + result + .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and + result != "equalsIgnoreCase" + } } -/** Gets a guard that depends on a flag. */ -private Guard getAGuard() { - exists(DataFlow::Node source, DataFlow::Node sink | - isFlag(source) and - flagFlowStep*(source, sink) and - sink.asExpr() = result - ) +/** 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 feature. */ +/** 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 = getAGuard()) + exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | + g = getASecurityFeatureFlagGuard() or g = getAnInsecureTrustManagerFlagGuard() + ) } from diff --git a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql index 5fce4a588ea7..6d68c6642d21 100644 --- a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql +++ b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql @@ -15,6 +15,7 @@ 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 private import semmle.code.java.dataflow.ExternalFlow @@ -86,76 +87,30 @@ private class HostnameVerifierSink extends DataFlow::Node { HostnameVerifierSink() { sinkNode(this, "set-hostname-verifier") } } -bindingset[result] -private string getAFlagName() { - result - .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") -} - /** - * A flag has to either be of type `String`, `boolean` or `Boolean`. + * Flags suggesting a deliberately unsafe `HostnameVerifier` usage. */ -private class FlagType extends Type { - FlagType() { - this instanceof TypeString - or - this instanceof BooleanType +private class UnsafeHostnameVerificationFlag extends FlagKind { + UnsafeHostnameVerificationFlag() { this = "UnsafeHostnameVerificationFlag" } + + bindingset[result] + override string getAFlagName() { + result + .regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*") and + result != "equalsIgnoreCase" } } -private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) { - ma.getMethod().hasName("equalsIgnoreCase") and - ma.getMethod().getDeclaringType() instanceof TypeString -} - -/** Holds if `source` should is considered a flag. */ -private predicate isFlag(DataFlow::Node source) { - exists(VarAccess v | v.getVariable().getName() = getAFlagName() | - source.asExpr() = v and v.getType() instanceof FlagType - ) - or - exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s) - or - exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | - source.asExpr() = ma and - ma.getType() instanceof FlagType and - not isEqualsIgnoreCaseMethodAccess(ma) - ) -} - -/** - * Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps: - * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. - * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. - * The return value of such a method is then tainted. - */ -private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - DataFlow::localFlowStep(node1, node2) - or - exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) | - ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() - ) - or - exists(MethodAccess ma | - ma.getMethod().hasName("parseBoolean") and - ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean") - | - ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() - ) +/** Gets a guard that represents a (likely) flag controlling an unsafe `HostnameVerifier` use. */ +private Guard getAnUnsafeHostnameVerifierFlagGuard() { + result = any(UnsafeHostnameVerificationFlag flag).getAFlag().asExpr() } -/** Gets a guard that depends on a flag. */ -private Guard getAGuard() { - exists(DataFlow::Node source, DataFlow::Node sink | - isFlag(source) and - flagFlowStep*(source, sink) and - sink.asExpr() = result - ) -} - -/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */ +/** 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 = getAGuard()) + exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | + g = getASecurityFeatureFlagGuard() or g = getAnUnsafeHostnameVerifierFlagGuard() + ) } from diff --git a/java/ql/src/semmle/code/java/security/SecurityFlag.qll b/java/ql/src/semmle/code/java/security/SecurityFlag.qll new file mode 100644 index 000000000000..cd6fe6ddd177 --- /dev/null +++ b/java/ql/src/semmle/code/java/security/SecurityFlag.qll @@ -0,0 +1,84 @@ +/** + * Provides utility predicates to spot variable names, parameter names, and string literals that suggest deliberately insecure settings. + */ + +import java +import semmle.code.java.controlflow.Guards +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.FlowSources + +/** + * A kind of flag that may indicate security expectations regarding the code it guards. + */ +abstract class FlagKind extends string { + bindingset[this] + FlagKind() { any() } + + /** + * Returns a flag name of this type. + */ + bindingset[result] + abstract string getAFlagName(); + + /** Gets a node representing a (likely) security flag. */ + DataFlow::Node getAFlag() { + exists(VarAccess v | v.getVariable().getName() = getAFlagName() | + result.asExpr() = v and v.getType() instanceof FlagType + ) + or + exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | result.asExpr() = s) + or + exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | + result.asExpr() = ma and + ma.getType() instanceof FlagType + ) + or + flagFlowStep*(getAFlag(), result) + } +} + +/** + * Flags suggesting an optional feature, perhaps deliberately insecure. + */ +class SecurityFeatureFlag extends FlagKind { + SecurityFeatureFlag() { this = "SecurityFeatureFlag" } + + bindingset[result] + override string getAFlagName() { result.regexpMatch("(?i).*(secure|(en|dis)able).*") } +} + +/** + * A flag has to either be of type `String`, `boolean` or `Boolean`. + */ +private class FlagType extends Type { + FlagType() { + this instanceof TypeString + or + this instanceof BooleanType + } +} + +/** + * Holds if there is local flow from `node1` to `node2` either due to standard data-flow steps or the + * following custom flow steps: + * 1. `Boolean.parseBoolean(taintedValue)` taints the return value of `parseBoolean`. + * 2. A call to an `EnvReadMethod` such as `System.getProperty` where a tainted value is used as an argument. + * The return value of such a method is then tainted. + */ +private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + DataFlow::localFlowStep(node1, node2) + or + exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) | + ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() + ) + or + exists(MethodAccess ma | + ma.getMethod().hasName("parseBoolean") and + ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean") + | + ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr() + ) +} + +/** Gets a guard that represents a (likely) security feature-flag check. */ +Guard getASecurityFeatureFlagGuard() { result = any(SecurityFeatureFlag flag).getAFlag().asExpr() } From 6bfdf8d148276e0e8df351fd9e41be5bbdebae6d Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 28 Apr 2021 22:11:43 +0200 Subject: [PATCH 20/28] Java: Fix qhelp errors. --- .../Security/CWE/CWE-295/InsecureTrustManager.qhelp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp index 5f7b4ec39ac1..d76927c050e0 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp @@ -13,14 +13,13 @@ An attack might look like this:

      -
    1. The vulnerable program connects to https://example.com. -
    2. The attacker intercepts this connection and presents a valid, self-signed certificate for https://example.com. -
    3. The vulnerable program calls the checkServerTrusted method to check whether it should trust the certificate. -
    4. The checkServerTrusted method of your TrustManager does not throw a CertificateException. -
    5. The vulnerable program accepts the certificate and proceeds with the connection since your TrustManager implicitly trusted it by not throwing an exception. -
    6. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure. +
    7. The vulnerable program connects to https://example.com.
      8. +
    8. The attacker intercepts this connection and presents a valid, self-signed certificate for https://example.com.
      9. +
    9. The vulnerable program calls the checkServerTrusted method to check whether it should trust the certificate.
      10. +
    10. The checkServerTrusted method of your TrustManager does not throw a CertificateException.
      11. +
    11. The vulnerable program accepts the certificate and proceeds with the connection since your TrustManager implicitly trusted it by not throwing an exception.
      12. +
    12. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure.
    -

    From f0d4b1d2b0bc5f72b1a6393c1fba03747046dbc7 Mon Sep 17 00:00:00 2001 From: intrigus Date: Tue, 4 May 2021 15:54:44 +0200 Subject: [PATCH 21/28] Java: Add change-note. --- java/change-notes/2021-05-04-insecure-trust-manager.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/change-notes/2021-05-04-insecure-trust-manager.md diff --git a/java/change-notes/2021-05-04-insecure-trust-manager.md b/java/change-notes/2021-05-04-insecure-trust-manager.md new file mode 100644 index 000000000000..7e78a63637d9 --- /dev/null +++ b/java/change-notes/2021-05-04-insecure-trust-manager.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* A new query "`TrustManager` that accepts all certificates" + (`java/insecure-trustmanager`) has been added. This query finds insecure + `TrustManager`s that may allow machine-in-the-middle attacks. From f527df73d56517a06c1a2d620f179313bff99068 Mon Sep 17 00:00:00 2001 From: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com> Date: Wed, 5 May 2021 23:39:49 +0200 Subject: [PATCH 22/28] Apply suggestions from code review. Co-authored-by: Felicity Chapman --- .../src/Security/CWE/CWE-295/InsecureTrustManager.qhelp | 9 ++++----- java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp index d76927c050e0..99746477a343 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp @@ -33,16 +33,15 @@ See below for an example of how to do this.

    -In the first (bad) example, the TrustManager never throws a CertificateException thereby trusting any certificate. +In the first (bad) example, the TrustManager never throws a CertificateException and therefore implicitly trusts any certificate. This allows an attacker to perform a machine-in-the-middle attack. -In the second (good) example, no custom TrustManager is used. Instead, the self-signed certificate that should be trusted -is explicitly trusted by loading it into a KeyStore. +In the second (good) example, the self-signed certificate that should be trusted +is loaded into a KeyStore. This explicitly defines the certificate as trusted and there is no need to create a custom TrustManager.

    -
  • Android Security Guide for TLS/HTTPS.
    • -
  • OWASP: CWE-295.
    • +
  • Android Develoers:Security with HTTPS and SSL.
    •  diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 866641d3cf1e..598113ed5cda 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -20,7 +20,7 @@ import DataFlow::PathGraph /** * An insecure `X509TrustManager`. * An `X509TrustManager` is considered insecure if it never throws a `CertificateException` - * thereby accepting any certificate as valid. + * and therefore implicitly trusts any certificate as valid. */ class InsecureX509TrustManager extends RefType { InsecureX509TrustManager() { From fe923facc8d2f511c13137d88252673e242be0e3 Mon Sep 17 00:00:00 2001 From: intrigus Date: Wed, 5 May 2021 23:41:55 +0200 Subject: [PATCH 23/28] Java: Move comments to separate lines. Move comments to separate lines to improve the rendering in the finished query help. --- .../src/Security/CWE/CWE-295/InsecureTrustManager.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java index bca408f9f013..4e9c3ce6bbc9 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java @@ -39,10 +39,12 @@ public void checkClientTrusted(X509Certificate[] chain, String authType) throws // 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. + 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()); From 36575bb26fcd02ff19deb82d57d91dbc2e8d4253 Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 17 Jun 2021 19:10:50 +0200 Subject: [PATCH 24/28] Move back to experimental......... --- .../Security/CWE/CWE-295/InsecureTrustManager.java | 0 .../Security/CWE/CWE-295/InsecureTrustManager.qhelp | 0 .../Security/CWE/CWE-295/InsecureTrustManager.ql | 0 .../query-tests/security/CWE-295/InsecureTrustManager.expected | 0 .../query-tests/security/CWE-295/InsecureTrustManager.qlref | 0 .../query-tests/security/CWE-295/InsecureTrustManagerTest.java | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename java/ql/src/{ => experimental}/Security/CWE/CWE-295/InsecureTrustManager.java (100%) rename java/ql/src/{ => experimental}/Security/CWE/CWE-295/InsecureTrustManager.qhelp (100%) rename java/ql/src/{ => experimental}/Security/CWE/CWE-295/InsecureTrustManager.ql (100%) rename java/ql/test/{ => experimental}/query-tests/security/CWE-295/InsecureTrustManager.expected (100%) rename java/ql/test/{ => experimental}/query-tests/security/CWE-295/InsecureTrustManager.qlref (100%) rename java/ql/test/{ => experimental}/query-tests/security/CWE-295/InsecureTrustManagerTest.java (100%) diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java b/java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.java similarity index 100% rename from java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java rename to java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.java diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp b/java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.qhelp similarity index 100% rename from java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp rename to java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.qhelp diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.ql similarity index 100% rename from java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql rename to java/ql/src/experimental/Security/CWE/CWE-295/InsecureTrustManager.ql diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected rename to java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.expected diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref similarity index 100% rename from java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref rename to java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManagerTest.java similarity index 100% rename from java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java rename to java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManagerTest.java From 5106aec319ce3b8f3980bd4be0513c0686cbdc0b Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 17 Jun 2021 19:16:00 +0200 Subject: [PATCH 25/28] Fix test location. --- .../query-tests/security/CWE-295/InsecureTrustManager.qlref | 1 - .../{ => InsecureTrustManager}/InsecureTrustManager.expected | 0 .../CWE-295/InsecureTrustManager/InsecureTrustManager.qlref | 1 + .../{ => InsecureTrustManager}/InsecureTrustManagerTest.java | 0 4 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref rename java/ql/test/experimental/query-tests/security/CWE-295/{ => InsecureTrustManager}/InsecureTrustManager.expected (100%) create mode 100644 java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.qlref rename java/ql/test/experimental/query-tests/security/CWE-295/{ => InsecureTrustManager}/InsecureTrustManagerTest.java (100%) diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref deleted file mode 100644 index 9bb2ecf04e83..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-295/InsecureTrustManager.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected similarity index 100% rename from java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager.expected rename to java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.qlref b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.qlref new file mode 100644 index 000000000000..9950f6276595 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-295/InsecureTrustManager.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.java similarity index 100% rename from java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManagerTest.java rename to java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.java From be57aeccf29729ce5f9ad50e68f63aa5f69f0a0f Mon Sep 17 00:00:00 2001 From: intrigus Date: Thu, 17 Jun 2021 19:16:16 +0200 Subject: [PATCH 26/28] Remove change-note. --- java/change-notes/2021-05-04-insecure-trust-manager.md | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 java/change-notes/2021-05-04-insecure-trust-manager.md diff --git a/java/change-notes/2021-05-04-insecure-trust-manager.md b/java/change-notes/2021-05-04-insecure-trust-manager.md deleted file mode 100644 index 7e78a63637d9..000000000000 --- a/java/change-notes/2021-05-04-insecure-trust-manager.md +++ /dev/null @@ -1,4 +0,0 @@ -lgtm,codescanning -* A new query "`TrustManager` that accepts all certificates" - (`java/insecure-trustmanager`) has been added. This query finds insecure - `TrustManager`s that may allow machine-in-the-middle attacks. From a79356e31632c5d9a098add7e1f78143bbe8513f Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 25 Jun 2021 12:16:54 +0200 Subject: [PATCH 27/28] Apply suggestions from code review --- .../code/java/security/SecurityFlag.qll | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/java/ql/src/semmle/code/java/security/SecurityFlag.qll b/java/ql/src/semmle/code/java/security/SecurityFlag.qll index cd6fe6ddd177..e4f47d95ec45 100644 --- a/java/ql/src/semmle/code/java/security/SecurityFlag.qll +++ b/java/ql/src/semmle/code/java/security/SecurityFlag.qll @@ -15,32 +15,34 @@ abstract class FlagKind extends string { FlagKind() { any() } /** - * Returns a flag name of this type. + * Gets a flag name of this type. */ bindingset[result] abstract string getAFlagName(); /** Gets a node representing a (likely) security flag. */ DataFlow::Node getAFlag() { - exists(VarAccess v | v.getVariable().getName() = getAFlagName() | - result.asExpr() = v and v.getType() instanceof FlagType + exists(DataFlow::Node flag | + exists(VarAccess v | v.getVariable().getName() = getAFlagName() | + flag.asExpr() = v and v.getType() instanceof FlagType + ) + or + exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | flag.asExpr() = s) + or + exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | + flag.asExpr() = ma and + ma.getType() instanceof FlagType + ) + | + flagFlowStep*(flag, result) ) - or - exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | result.asExpr() = s) - or - exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() | - result.asExpr() = ma and - ma.getType() instanceof FlagType - ) - or - flagFlowStep*(getAFlag(), result) } } /** * Flags suggesting an optional feature, perhaps deliberately insecure. */ -class SecurityFeatureFlag extends FlagKind { +private class SecurityFeatureFlag extends FlagKind { SecurityFeatureFlag() { this = "SecurityFeatureFlag" } bindingset[result] From 5aa711a95626b06efa723c83700dd5c5d6a4f0fb Mon Sep 17 00:00:00 2001 From: intrigus Date: Fri, 25 Jun 2021 17:04:36 +0200 Subject: [PATCH 28/28] Accept test changes. --- .../InsecureTrustManager.expected | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected index cba1d7ede75c..08c8962cd983 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManager.expected @@ -1,57 +1,93 @@ edges -| InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:122:22:122:33 | trustManager | -| InsecureTrustManagerTest.java:130:55:130:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:131:23:131:34 | trustManager | -| InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:152:23:152:34 | trustManager | -| InsecureTrustManagerTest.java:172:55:172:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:173:23:173:34 | trustManager | -| InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:194:23:194:34 | trustManager | -| InsecureTrustManagerTest.java:214:55:214:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:215:23:215:34 | trustManager | -| InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:236:23:236:34 | trustManager | -| InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:258:23:258:34 | trustManager | -| InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:281:23:281:34 | trustManager | -| InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:306:22:306:33 | trustManager | -| InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:320:22:320:33 | trustManager | -| InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:334:22:334:33 | trustManager | -| InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:348:22:348:33 | trustManager | -| InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:362:22:362:33 | trustManager | -| InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:376:22:376:33 | trustManager | -| InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:391:22:391:33 | trustManager | -| InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:406:22:406:33 | trustManager | -| InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:415:22:415:33 | trustManager | +| InsecureTrustManagerTest.java:121:33:121:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:122:22:122:33 | trustManager | +| InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:121:33:121:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:130:34:130:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:131:23:131:34 | trustManager | +| InsecureTrustManagerTest.java:130:55:130:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:130:34:130:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:151:34:151:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:152:23:152:34 | trustManager | +| InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:151:34:151:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:172:34:172:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:173:23:173:34 | trustManager | +| InsecureTrustManagerTest.java:172:55:172:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:172:34:172:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:193:34:193:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:194:23:194:34 | trustManager | +| InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:193:34:193:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:214:34:214:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:215:23:215:34 | trustManager | +| InsecureTrustManagerTest.java:214:55:214:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:214:34:214:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:235:34:235:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:236:23:236:34 | trustManager | +| InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:235:34:235:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:257:34:257:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:258:23:258:34 | trustManager | +| InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:257:34:257:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:280:34:280:82 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:281:23:281:34 | trustManager | +| InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:280:34:280:82 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:305:33:305:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:306:22:306:33 | trustManager | +| InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:305:33:305:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:319:33:319:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:320:22:320:33 | trustManager | +| InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:319:33:319:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:333:33:333:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:334:22:334:33 | trustManager | +| InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:333:33:333:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:347:33:347:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:348:22:348:33 | trustManager | +| InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:347:33:347:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:361:33:361:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:362:22:362:33 | trustManager | +| InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:361:33:361:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:375:33:375:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:376:22:376:33 | trustManager | +| InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:375:33:375:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:390:33:390:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:391:22:391:33 | trustManager | +| InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:390:33:390:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:405:33:405:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:406:22:406:33 | trustManager | +| InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:405:33:405:81 | {...} [[]] : InsecureTrustManager | +| InsecureTrustManagerTest.java:414:33:414:81 | {...} [[]] : InsecureTrustManager | InsecureTrustManagerTest.java:415:22:415:33 | trustManager | +| InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:414:33:414:81 | {...} [[]] : InsecureTrustManager | nodes +| InsecureTrustManagerTest.java:121:33:121:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:121:54:121:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:122:22:122:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:130:34:130:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:130:55:130:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:131:23:131:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:151:34:151:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:151:55:151:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:152:23:152:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:172:34:172:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:172:55:172:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:173:23:173:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:193:34:193:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:193:55:193:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:194:23:194:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:214:34:214:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:214:55:214:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:215:23:215:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:235:34:235:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:235:55:235:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:236:23:236:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:257:34:257:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:257:55:257:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:258:23:258:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:280:34:280:82 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:280:55:280:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:281:23:281:34 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:305:33:305:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:305:54:305:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:306:22:306:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:319:33:319:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:319:54:319:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:320:22:320:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:333:33:333:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:333:54:333:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:334:22:334:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:347:33:347:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:347:54:347:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:348:22:348:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:361:33:361:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:361:54:361:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:362:22:362:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:375:33:375:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:375:54:375:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:376:22:376:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:390:33:390:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:390:54:390:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:391:22:391:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:405:33:405:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:405:54:405:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:406:22:406:33 | trustManager | semmle.label | trustManager | +| InsecureTrustManagerTest.java:414:33:414:81 | {...} [[]] : InsecureTrustManager | semmle.label | {...} [[]] : InsecureTrustManager | | InsecureTrustManagerTest.java:414:54:414:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager | | InsecureTrustManagerTest.java:415:22:415:33 | trustManager | semmle.label | trustManager | #select