diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.java b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.java new file mode 100644 index 000000000000..4d1a1b89aa9c --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.java @@ -0,0 +1,41 @@ +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 + } + }; + } +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.qhelp b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.qhelp new file mode 100644 index 000000000000..d526c024b67f --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.qhelp @@ -0,0 +1,31 @@ + + + + +

Java offers two mechanisms for SSL authentication - trust manager and hostname verifier. 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.

+

Unsafe implementation of the interface X509TrustManager, HostnameVerifier, 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, the hostname verifier is turned off, or setEndpointIdentificationAlgorithm is missing. The query also covers a special implementation com.rabbitmq.client.ConnectionFactory.

+ + + +

Validate SSL certificate in SSL authentication.

+ + + +

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

+ + + + +
  • +CWE-295 +
    • +
  • +How to fix apps containing an unsafe implementation of TrustManager +
    • +     +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.ql b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.ql new file mode 100644 index 000000000000..6ac985b3dd10 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.ql @@ -0,0 +1,58 @@ +/** + * @name Unsafe certificate trust + * @description Unsafe implementation of a `X509TrustManager`, thereby making the app vulnerable to man-in-the-middle attacks. + * @kind problem + * @id java/unsafe-cert-trust + * @tags security + * external/cwe-295 + */ + +import java +import semmle.code.java.security.Encryption + +/** + * A `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); + ) + ) + } +} + +from MethodAccess ma +where ma instanceof X509TrustAllManagerInit +select ma, "Unsafe configuration of trusted certificates" diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/experimental/Security/CWE/CWE-297/MissingSslEndpointIdentification.ql similarity index 66% rename from java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql rename to java/ql/src/experimental/Security/CWE/CWE-297/MissingSslEndpointIdentification.ql index 497e87b37ac3..1984319ac220 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-297/MissingSslEndpointIdentification.ql @@ -10,85 +10,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); - ) - ) - } -} - -/** - * HostnameVerifier class that allows a certificate whose CN (Common Name) does not match the host name in the URL - */ -class TrustAllHostnameVerifier extends RefType { - TrustAllHostnameVerifier() { - this.getASupertype*() instanceof HostnameVerifier and - exists(Method m, ReturnStmt rt | - m.getDeclaringType() = this and - m.hasName("verify") and - rt.getEnclosingCallable() = m and - rt.getResult().(BooleanLiteral).getBooleanValue() = true - ) - } -} - -/** - * The setDefaultHostnameVerifier method of HttpsURLConnection with the trust all configuration - */ -class TrustAllHostnameVerify extends MethodAccess { - TrustAllHostnameVerify() { - this.getMethod().hasName("setDefaultHostnameVerifier") and - this.getMethod().getDeclaringType() instanceof HttpsURLConnection and //httpsURLConnection.setDefaultHostnameVerifier method - ( - exists(NestedClass nc | - nc.getASupertype*() instanceof TrustAllHostnameVerifier and - this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...}); - ) - or - exists(Variable v | - this.getArgument(0).(VarAccess).getVariable() = v and - v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier); - ) - ) - } -} - class SSLEngine extends RefType { SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") } } @@ -239,8 +160,6 @@ class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess { from MethodAccess aa where - aa instanceof TrustAllHostnameVerify or - aa instanceof X509TrustAllManagerInit or aa instanceof SSLEndpointIdentificationNotSet or aa instanceof RabbitMQEnableHostnameVerificationNotSet select aa, "Unsafe configuration of trusted certificates" diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java b/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.java similarity index 67% rename from java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java rename to java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.java index 65698ac33a30..b5999a9fb866 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java +++ b/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.java @@ -26,46 +26,6 @@ public boolean verify(String hostname, SSLSession session) { HttpsURLConnection.setDefaultHostnameVerifier(verifier); } - { - 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-297/UnsafeHostnameVerification.qhelp similarity index 91% rename from java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp rename to java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp index 2e8d08fd68b9..07c9e3122c7b 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp @@ -22,10 +22,10 @@ no validation is performed thus any certificate is trusted. In the 'GOOD' case,
  • -CWE-273 +CWE-297
  • -How to fix apps containing an unsafe implementation of TrustManager +How to resolve Insecure HostnameVerifier
  • Testing Endpoint Identify Verification (MSTG-NETWORK-3) diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.ql b/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.ql new file mode 100644 index 000000000000..b4f5cdbe7cd0 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.ql @@ -0,0 +1,75 @@ +/** + * @name Unsafe certificate trust and improper hostname verification + * @description Unsafe implementation of the interface X509TrustManager, HostnameVerifier, 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. + * @kind problem + * @id java/unsafe-cert-trust + * @tags security + * external/cwe-273 + */ + +import java +import semmle.code.java.security.Encryption +import semmle.code.java.dataflow.DataFlow + +/** + * Holds if `m` always returns `true` ignoring any exceptional flow. + */ +private predicate alwaysReturnsTrue(HostnameVerifierVerify m) { + forex(ReturnStmt rs | rs.getEnclosingCallable() = m | + rs.getResult().(CompileTimeConstantExpr).getBooleanValue() = true + ) +} + +/** + * A class that overrides the `javax.net.ssl.HostnameVerifier.verify` method and **always** returns `true`, thus + * accepting any certificate despite a hostname mismatch. + */ +class TrustAllHostnameVerifier extends RefType { + TrustAllHostnameVerifier() { + this.getASupertype*() instanceof HostnameVerifier and + exists(HostnameVerifierVerify m | + m.getDeclaringType() = this and + alwaysReturnsTrue(m) + ) + } +} + +class SSLEngine extends RefType { + SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") } +} + +class Socket extends RefType { + Socket() { this.hasQualifiedName("java.net", "Socket") } +} + +class SocketFactory extends RefType { + SocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") } +} + +class SSLSocket extends RefType { + SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") } +} + +/** + * A configuration to model the flow of a `TrustAllHostnameVerifier` to a `set(Default)HostnameVerifier` call. + */ +class TrustAllHostnameVerifierConfiguration extends DataFlow::Configuration { + TrustAllHostnameVerifierConfiguration() { this = "TrustAllHostnameVerifierConfiguration" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof TrustAllHostnameVerifier + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma, Method m | + (m instanceof SetDefaultHostnameVerifierMethod or m instanceof SetHostnameVerifierMethod) and + ma.getMethod() = m + | + ma.getArgument(0) = sink.asExpr() + ) + } +} + +from DataFlow::Node source, DataFlow::Node sink, TrustAllHostnameVerifierConfiguration cfg +where cfg.hasFlow(source, sink) +select sink, "Unsafe configuration of trusted certificates" diff --git a/java/ql/src/semmle/code/java/security/Encryption.qll b/java/ql/src/semmle/code/java/security/Encryption.qll index ea7a33151f8f..961153cc5752 100644 --- a/java/ql/src/semmle/code/java/security/Encryption.qll +++ b/java/ql/src/semmle/code/java/security/Encryption.qll @@ -67,6 +67,13 @@ class SetConnectionFactoryMethod extends Method { } } +class SetDefaultHostnameVerifierMethod extends Method { + SetDefaultHostnameVerifierMethod() { + hasName("setDefaultHostnameVerifier") and + getDeclaringType().getASupertype*() instanceof HttpsURLConnection + } +} + class SetHostnameVerifierMethod extends Method { SetHostnameVerifierMethod() { hasName("setHostnameVerifier") and 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 deleted file mode 100644 index c0ea40f9bdb0..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected +++ /dev/null @@ -1,7 +0,0 @@ -| 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:55:3:60:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:73:3:73:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:124:25:124:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:135:25:135:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | -| UnsafeCertTrustTest.java:144:34:144:83 | createSocket(...) | Unsafe configuration of trusted certificates | diff --git a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.qlref b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.qlref deleted file mode 100644 index f054d6037876..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.expected b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.expected new file mode 100644 index 000000000000..f0ca58390ffb --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.expected @@ -0,0 +1,2 @@ +| UnsafeCertificateTrust.java:27:4:27:74 | init(...) | Unsafe configuration of trusted certificates | +| UnsafeCertificateTrust.java:42:4:42:38 | init(...) | Unsafe configuration of trusted certificates | diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.java b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.java new file mode 100644 index 000000000000..e8bcc0f8dd5a --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.java @@ -0,0 +1,87 @@ +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + +import java.net.Socket; +import javax.net.SocketFactory; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; + +//import com.rabbitmq.client.ConnectionFactory; + +public class UnsafeCertificateTrust { + + /** + * 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 + } + }; + +} \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.qlref b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.qlref new file mode 100644 index 000000000000..bad00eae8b33 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-295/UnsafeCertificateTrust.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-295/UnsafeCertificateTrust.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.expected b/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.expected new file mode 100644 index 000000000000..20bb5a58b8b3 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.expected @@ -0,0 +1,5 @@ +| UnsafeHostnameVerification.java:25:3:30:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates | +| UnsafeHostnameVerification.java:43:3:43:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates | +| UnsafeHostnameVerification.java:58:25:58:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | +| UnsafeHostnameVerification.java:69:25:69:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates | +| UnsafeHostnameVerification.java:78:34:78:83 | createSocket(...) | 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-297/UnsafeHostnameVerification.java similarity index 60% rename from java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java rename to java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.java index ff62035fd333..17fab6d1199d 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java +++ b/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.java @@ -16,37 +16,7 @@ //import com.rabbitmq.client.ConnectionFactory; -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); - } - } +public class UnsafeHostnameVerification { /** * Test the implementation of trusting all hostnames as an anonymous class @@ -60,6 +30,13 @@ public boolean verify(String hostname, SSLSession session) { }); } + /** + * Test the implementation of trusting all hostnames as a lambda. + */ + public void testTrustAllHostnameLambda() { + HttpsURLConnection.setDefaultHostnameVerifier((name, s) -> true); + } + /** * Test the implementation of trusting all hostnames as a variable */ @@ -73,42 +50,6 @@ public boolean verify(String hostname, SSLSession session) { HttpsURLConnection.setDefaultHostnameVerifier(verifier); } - 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 - } - }; - public static final HostnameVerifier ALLOW_ALL_HOSTNAME_VERIFIER = new HostnameVerifier() { @Override public boolean verify(String hostname, SSLSession session) { diff --git a/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.qlref b/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.qlref new file mode 100644 index 000000000000..c0c80eedc6d3 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-297/UnsafeHostnameVerification.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-297/UnsafeHostnameVerification.ql