diff --git a/java/ql/lib/semmle/code/java/frameworks/Networking.qll b/java/ql/lib/semmle/code/java/frameworks/Networking.qll index 6e5ad743d44d..daa1f4ca8f96 100644 --- a/java/ql/lib/semmle/code/java/frameworks/Networking.qll +++ b/java/ql/lib/semmle/code/java/frameworks/Networking.qll @@ -14,6 +14,11 @@ class TypeSocket extends RefType { TypeSocket() { this.hasQualifiedName("java.net", "Socket") } } +/** The type `javax.net.SocketFactory` */ +class TypeSocketFactory extends RefType { + TypeSocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") } +} + /** The type `java.net.URL`. */ class TypeUrl extends RefType { TypeUrl() { this.hasQualifiedName("java.net", "URL") } @@ -42,6 +47,15 @@ class SocketGetInputStreamMethod extends Method { } } +/** The method `java.net.Socket::getOutputStream`. */ +class SocketGetOutputStreamMethod extends Method { + SocketGetOutputStreamMethod() { + this.getDeclaringType() instanceof TypeSocket and + this.hasName("getOutputStream") and + this.hasNoParameters() + } +} + /** A method or constructor call that returns a new `URI`. */ class UriCreation extends Call { UriCreation() { @@ -143,6 +157,22 @@ class UrlOpenConnectionMethod extends Method { } } +/** The method `javax.net.SocketFactory::createSocket`. */ +class CreateSocketMethod extends Method { + CreateSocketMethod() { + this.hasName("createSocket") and + this.getDeclaringType().getASupertype*() instanceof TypeSocketFactory + } +} + +/** The method `javax.net.Socket::connect`. */ +class SocketConnectMethod extends Method { + SocketConnectMethod() { + this.hasName("connect") and + this.getDeclaringType() instanceof TypeSocket + } +} + /** * A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary. * Several examples are localhost, reserved IPv4 IP addresses including 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses including [0:0:0:0:0:0:0:1] and [::1] diff --git a/java/ql/lib/semmle/code/java/security/Encryption.qll b/java/ql/lib/semmle/code/java/security/Encryption.qll index 8f4cef23ee6c..513c4c0099ca 100644 --- a/java/ql/lib/semmle/code/java/security/Encryption.qll +++ b/java/ql/lib/semmle/code/java/security/Encryption.qll @@ -34,6 +34,21 @@ class SSLSession extends RefType { SSLSession() { this.hasQualifiedName("javax.net.ssl", "SSLSession") } } +/** The `javax.net.ssl.SSLEngine` class. */ +class SSLEngine extends RefType { + SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") } +} + +/** The `javax.net.ssl.SSLSocket` class. */ +class SSLSocket extends RefType { + SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") } +} + +/** The `javax.net.ssl.SSLParameters` class. */ +class SSLParameters extends RefType { + SSLParameters() { this.hasQualifiedName("javax.net.ssl", "SSLParameters") } +} + class HostnameVerifier extends RefType { HostnameVerifier() { this.hasQualifiedName("javax.net.ssl", "HostnameVerifier") } } @@ -79,6 +94,14 @@ class GetSocketFactory extends Method { } } +/** The `createSSLEngine` method of the class `javax.net.ssl.SSLContext` */ +class CreateSslEngineMethod extends Method { + CreateSslEngineMethod() { + this.hasName("createSSLEngine") and + this.getDeclaringType() instanceof SSLContext + } +} + class SetConnectionFactoryMethod extends Method { SetConnectionFactoryMethod() { this.hasName("setSSLSocketFactory") and @@ -101,6 +124,38 @@ class SetDefaultHostnameVerifierMethod extends Method { } } +/** The `beginHandshake` method of the class `javax.net.ssl.SSLEngine`. */ +class BeginHandshakeMethod extends Method { + BeginHandshakeMethod() { + this.hasName("beginHandshake") and + this.getDeclaringType().getASupertype*() instanceof SSLEngine + } +} + +/** The `wrap` method of the class `javax.net.ssl.SSLEngine`. */ +class SslWrapMethod extends Method { + SslWrapMethod() { + this.hasName("wrap") and + this.getDeclaringType().getASupertype*() instanceof SSLEngine + } +} + +/** The `unwrap` method of the class `javax.net.ssl.SSLEngine`. */ +class SslUnwrapMethod extends Method { + SslUnwrapMethod() { + this.hasName("unwrap") and + this.getDeclaringType().getASupertype*() instanceof SSLEngine + } +} + +/** The `getSession` method of the class `javax.net.ssl.SSLSession`.select */ +class GetSslSessionMethod extends Method { + GetSslSessionMethod() { + hasName("getSession") and + getDeclaringType().getASupertype*() instanceof SSLSession + } +} + bindingset[algorithmString] private string algorithmRegex(string algorithmString) { // Algorithms usually appear in names surrounded by characters that are not diff --git a/java/ql/lib/semmle/code/java/security/UnsafeCertTrust.qll b/java/ql/lib/semmle/code/java/security/UnsafeCertTrust.qll new file mode 100644 index 000000000000..c94864d86859 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/UnsafeCertTrust.qll @@ -0,0 +1,98 @@ +/** Provides classes and predicates to reason about unsafe certificate trust vulnerablities. */ + +import java +private import semmle.code.java.frameworks.Networking +private import semmle.code.java.security.Encryption +private import semmle.code.java.dataflow.DataFlow + +/** + * The creation of an object that prepares an SSL connection. + * + * This is a source for `SslEndpointIdentificationFlowConfig`. + */ +class SslConnectionInit extends DataFlow::Node { + SslConnectionInit() { + exists(MethodAccess ma, Method m | + this.asExpr() = ma and + ma.getMethod() = m + | + m instanceof CreateSslEngineMethod + or + m instanceof CreateSocketMethod and isSslSocket(ma) + ) + } +} + +/** + * A call to a method that establishes an SSL connection. + * + * This is a sink for `SslEndpointIdentificationFlowConfig`. + */ +class SslConnectionCreation extends DataFlow::Node { + SslConnectionCreation() { + exists(MethodAccess ma, Method m | + m instanceof BeginHandshakeMethod or + m instanceof SslWrapMethod or + m instanceof SslUnwrapMethod or + m instanceof SocketGetOutputStreamMethod + | + ma.getMethod() = m and + this.asExpr() = ma.getQualifier() + ) + } +} + +/** + * An SSL object that correctly verifies hostnames, or doesn't need to (for instance, because it's a server). + * + * This is a sanitizer for `SslEndpointIdentificationFlowConfig`. + */ +abstract class SslUnsafeCertTrustSanitizer extends DataFlow::Node { } + +/** + * An `SSLEngine` set in server mode. + */ +private class SslEngineServerMode extends SslUnsafeCertTrustSanitizer { + SslEngineServerMode() { + exists(MethodAccess ma, Method m | + m.hasName("setUseClientMode") and + m.getDeclaringType().getASupertype*() instanceof SSLEngine and + ma.getMethod() = m and + ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = false and + this.asExpr() = ma.getQualifier() + ) + } +} + +/** + * Holds if the return value of `createSocket` is cast to `SSLSocket` + * or the qualifier of `createSocket` is an instance of `SSLSocketFactory`. + */ +private predicate isSslSocket(MethodAccess createSocket) { + createSocket = any(CastExpr ce | ce.getType() instanceof SSLSocket).getExpr() + or + createSocket.getQualifier().getType().(RefType).getASupertype*() instanceof SSLSocketFactory +} + +/** + * A call to a method that enables SSL (`useSslProtocol` or `setSslContextFactory`) + * on an instance of `com.rabbitmq.client.ConnectionFactory` that doesn't set `enableHostnameVerification`. + */ +class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess { + RabbitMQEnableHostnameVerificationNotSet() { + this.getMethod().hasName(["useSslProtocol", "setSslContextFactory"]) and + this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and + exists(Variable v | + v.getType() instanceof RabbitMQConnectionFactory and + this.getQualifier() = v.getAnAccess() and + not exists(MethodAccess ma | + ma.getMethod().hasName("enableHostnameVerification") and + ma.getQualifier() = v.getAnAccess() + ) + ) + } +} + +private class RabbitMQConnectionFactory extends RefType { + RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") } +} diff --git a/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll new file mode 100644 index 000000000000..767b86d4e412 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll @@ -0,0 +1,65 @@ +/** Provides taint tracking configurations to be used by unsafe certificate trust queries. */ + +import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.security.UnsafeCertTrust +import semmle.code.java.security.Encryption + +/** + * A taint flow configuration for SSL connections created without a proper certificate trust configuration. + */ +class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { + SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit } + + override predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + sanitizer instanceof SslUnsafeCertTrustSanitizer + } +} + +/** + * An SSL object that was assigned a safe `SSLParameters` object and can be considered safe. + */ +private class SslConnectionWithSafeSslParameters extends SslUnsafeCertTrustSanitizer { + SslConnectionWithSafeSslParameters() { + exists(SafeSslParametersFlowConfig config, DataFlow::Node safe, DataFlow::Node sanitizer | + config.hasFlowTo(safe) and + sanitizer = DataFlow::exprNode(safe.asExpr().(Argument).getCall().getQualifier()) and + DataFlow::localFlow(sanitizer, this) + ) + } +} + +private class SafeSslParametersFlowConfig extends DataFlow2::Configuration { + SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + exists(MethodAccess ma | + ma instanceof SafeSetEndpointIdentificationAlgorithm and + DataFlow::getInstanceArgument(ma) = source.(DataFlow::PostUpdateNode).getPreUpdateNode() + ) + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma, RefType t | t instanceof SSLSocket or t instanceof SSLEngine | + ma.getMethod().hasName("setSSLParameters") and + ma.getMethod().getDeclaringType().getASupertype*() = t and + ma.getArgument(0) = sink.asExpr() + ) + } +} + +/** + * A call to `SSLParameters.setEndpointIdentificationAlgorithm` with a non-null and non-empty parameter. + */ +private class SafeSetEndpointIdentificationAlgorithm extends MethodAccess { + SafeSetEndpointIdentificationAlgorithm() { + this.getMethod().hasName("setEndpointIdentificationAlgorithm") and + this.getMethod().getDeclaringType() instanceof SSLParameters and + not this.getArgument(0) instanceof NullLiteral and + not this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "" + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.java rename to java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.java diff --git a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.qhelp b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.qhelp new file mode 100644 index 000000000000..8545b3ffbb6d --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.qhelp @@ -0,0 +1,50 @@ + + + + +

Java offers two mechanisms for SSL authentication - trust manager and hostname verifier (the later is checked by the java/insecure-hostname-verifier query). The 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.

+

When SSLSocket or SSLEngine are created without a secure setEndpointIdentificationAlgorithm, hostname verification is disabled by default.

+

This query checks whether setEndpointIdentificationAlgorithm is missing, thereby making the application vulnerable to man-in-the-middle attacks. The query also covers insecure configurations of com.rabbitmq.client.ConnectionFactory.

+ + + +

Validate SSL certificates in SSL authentication.

+ + + +

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.

+ + + + +
  • +Testing Endpoint Identify Verification (MSTG-NETWORK-3). +
    • +
  • + SSLParameters.setEndpointIdentificationAlgorithm documentation. +
    • +
  • + RabbitMQ: + ConnectionFactory.enableHostnameVerification documentation. +
    • +
  • + RabbitMQ: + Using TLS in the Java Client. +
    • +
  • +CVE-2018-17187: Apache Qpid Proton-J transport issue with hostname verification. +
    • +
  • +CVE-2018-8034: Apache Tomcat - host name verification when using TLS with the WebSocket client. +
    • +
  • +CVE-2018-11087: Pivotal Spring AMQP vulnerability due to lack of hostname validation. +
    • +
  • +CVE-2018-11775: TLS hostname verification issue when using the Apache ActiveMQ Client. +
    • +     +     diff --git a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql new file mode 100644 index 000000000000..13e1375d164a --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql @@ -0,0 +1,23 @@ +/** + * @name Unsafe certificate trust + * @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 + * @problem.severity warning + * @precision medium + * @id java/unsafe-cert-trust + * @tags security + * external/cwe/cwe-273 + */ + +import java +import semmle.code.java.security.UnsafeCertTrustQuery + +from Expr unsafeTrust +where + unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet or + exists(SslEndpointIdentificationFlowConfig config | + config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) + ) +select unsafeTrust, "Unsafe configuration of trusted certificates." diff --git a/java/ql/src/change-notes/2021-06-28-unsafe-cert-trust-query.md b/java/ql/src/change-notes/2021-06-28-unsafe-cert-trust-query.md new file mode 100644 index 000000000000..bde0c9d02496 --- /dev/null +++ b/java/ql/src/change-notes/2021-06-28-unsafe-cert-trust-query.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* The query "Unsafe certificate trust" (`java/unsafe-cert-trust`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3550). diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp deleted file mode 100644 index ae8d76b1bb1f..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp +++ /dev/null @@ -1,42 +0,0 @@ - - - - -

    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 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 SSLSocket/SSLEngine. In the 'BAD' case, -setEndpointIdentificationAlgorithm is not called, thus no hostname verification takes place. In the 'GOOD' case, setEndpointIdentificationAlgorithm is called.

    - -     - - -
  • -CWE-273 -
    • -
  • -Testing Endpoint Identify Verification (MSTG-NETWORK-3) -
    • -
  • -CVE-2018-17187: Apache Qpid Proton-J transport issue with hostname verification -
    • -
  • -CVE-2018-8034: Apache Tomcat - host name verification when using TLS with the WebSocket client -
    • -
  • -CVE-2018-11087: Pivotal Spring AMQP vulnerability due to lack of hostname validation -
    • -
  • -CVE-2018-11775: TLS hostname verification issue when using the Apache ActiveMQ Client -
    • -     -     diff --git a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql deleted file mode 100644 index a86505606ca0..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql +++ /dev/null @@ -1,169 +0,0 @@ -/** - * @name Unsafe certificate trust - * @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 - * @problem.severity warning - * @precision medium - * @id java/unsafe-cert-trust - * @tags security - * external/cwe/cwe-273 - */ - -import java -import semmle.code.java.security.Encryption - -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") } -} - -/** - * has setEndpointIdentificationAlgorithm set correctly - */ -predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) { - exists( - Variable sslo, MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set - | - createSSL = sslo.getAnAssignedValue() and - ma.getQualifier() = sslo.getAnAccess() and - ma.getMethod().hasName("setSSLParameters") and - ma.getArgument(0) = sslparams.getAnAccess() and - exists(MethodAccess setepa | - setepa.getQualifier() = sslparams.getAnAccess() and - setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and - not setepa.getArgument(0) instanceof NullLiteral - ) - ) -} - -/** - * has setEndpointIdentificationAlgorithm set correctly - */ -predicate hasEndpointIdentificationAlgorithm(Variable ssl) { - exists( - MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set - | - ma.getQualifier() = ssl.getAnAccess() and - ma.getMethod().hasName("setSSLParameters") and - ma.getArgument(0) = sslparams.getAnAccess() and - exists(MethodAccess setepa | - setepa.getQualifier() = sslparams.getAnAccess() and - setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and - not setepa.getArgument(0) instanceof NullLiteral - ) - ) -} - -/** - * Cast of Socket to SSLSocket - */ -predicate sslCast(MethodAccess createSSL) { - exists(Variable ssl, CastExpr ce | - ce.getExpr() = createSSL and - ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and - ssl.getType() instanceof SSLSocket //With a type cast `SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443)` - ) -} - -/** - * SSL object is created in a separate method call or in the same method - */ -predicate hasFlowPath(MethodAccess createSSL, Variable ssl) { - ( - createSSL = ssl.getAnAssignedValue() - or - exists(CastExpr ce | - ce.getExpr() = createSSL and - ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443); - ) - ) - or - exists(MethodAccess tranm | - createSSL.getEnclosingCallable() = tranm.getMethod() and - tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and - not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method - ) -} - -/** - * Not have the SSLParameter set - */ -predicate hasNoEndpointIdentificationSet(MethodAccess createSSL, Variable ssl) { - //No setSSLParameters set - hasFlowPath(createSSL, ssl) and - not exists(MethodAccess ma | - ma.getQualifier() = ssl.getAnAccess() and - ma.getMethod().hasName("setSSLParameters") - ) - or - //No endpointIdentificationAlgorithm set with setSSLParameters - hasFlowPath(createSSL, ssl) and - not setEndpointIdentificationAlgorithm(createSSL) -} - -/** - * The setEndpointIdentificationAlgorithm method of SSLParameters with the ssl engine or socket - */ -class SSLEndpointIdentificationNotSet extends MethodAccess { - SSLEndpointIdentificationNotSet() { - ( - this.getMethod().hasName("createSSLEngine") and - this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext - or - this.getMethod().hasName("createSocket") and - this.getMethod().getDeclaringType() instanceof SocketFactory and - this.getMethod().getReturnType() instanceof Socket and - sslCast(this) //createSocket method of SocketFactory - ) and - exists(Variable ssl | - hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself - not exists(VariableAssign ar, Variable newSsl | - ar.getSource() = this.getCaller().getAReference() and - ar.getDestVar() = newSsl and - hasEndpointIdentificationAlgorithm(newSsl) //Not set in its caller either - ) - ) and - not exists(MethodAccess ma | ma.getMethod() instanceof HostnameVerifierVerify) //Reduce false positives since this method access set default hostname verifier - } -} - -class RabbitMQConnectionFactory extends RefType { - RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") } -} - -/** - * The com.rabbitmq.client.ConnectionFactory useSslProtocol method access without enableHostnameVerification - */ -class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess { - RabbitMQEnableHostnameVerificationNotSet() { - this.getMethod().hasName("useSslProtocol") and - this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and - exists(Variable v | - v.getType() instanceof RabbitMQConnectionFactory and - this.getQualifier() = v.getAnAccess() and - not exists(MethodAccess ma | - ma.getMethod().hasName("enableHostnameVerification") and - ma.getQualifier() = v.getAnAccess() - ) - ) - } -} - -from MethodAccess aa -where - aa instanceof SSLEndpointIdentificationNotSet or - aa instanceof RabbitMQEnableHostnameVerificationNotSet -select aa, "Unsafe configuration of trusted certificates" diff --git a/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll b/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll index bfa2530b07e7..7ca794220fbf 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-327/SslLib.qll @@ -94,18 +94,6 @@ class UnsafeTlsVersion extends StringLiteral { } } -class SSLParameters extends RefType { - SSLParameters() { hasQualifiedName("javax.net.ssl", "SSLParameters") } -} - -class SSLSocket extends RefType { - SSLSocket() { hasQualifiedName("javax.net.ssl", "SSLSocket") } -} - class SSLServerSocket extends RefType { SSLServerSocket() { hasQualifiedName("javax.net.ssl", "SSLServerSocket") } } - -class SSLEngine extends RefType { - SSLEngine() { hasQualifiedName("javax.net.ssl", "SSLEngine") } -} 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 f26706a56d2c..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected +++ /dev/null @@ -1,3 +0,0 @@ -| 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 | 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-273/UnsafeCertTrustTest.java b/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java deleted file mode 100644 index cb8b472eb8fd..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java +++ /dev/null @@ -1,64 +0,0 @@ -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 UnsafeCertTrustTest { - - /** - * Test the endpoint identification of SSL engine is set to null - */ - public void testSSLEngineEndpointIdSetNull() throws java.security.NoSuchAlgorithmException { - SSLContext sslContext = SSLContext.getInstance("TLS"); - SSLEngine sslEngine = sslContext.createSSLEngine(); - SSLParameters sslParameters = sslEngine.getSSLParameters(); - sslParameters.setEndpointIdentificationAlgorithm(null); - sslEngine.setSSLParameters(sslParameters); - } - - /** - * Test the endpoint identification of SSL engine is not set - */ - public void testSSLEngineEndpointIdNotSet() throws java.security.NoSuchAlgorithmException { - SSLContext sslContext = SSLContext.getInstance("TLS"); - SSLEngine sslEngine = sslContext.createSSLEngine(); - } - - /** - * Test the endpoint identification of SSL socket is not set - */ - public void testSSLSocketEndpointIdNotSet() throws java.security.NoSuchAlgorithmException, java.io.IOException { - SSLContext sslContext = SSLContext.getInstance("TLS"); - final SSLSocketFactory socketFactory = sslContext.getSocketFactory(); - SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443); - } - - /** - * Test the endpoint identification of regular socket is not set - */ - public void testSocketEndpointIdNotSet() throws java.io.IOException { - SocketFactory socketFactory = SocketFactory.getDefault(); - Socket socket = socketFactory.createSocket("www.example.com", 80); - } - - // /** - // * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set - // */ - // public void testEnableHostnameVerificationOfRabbitMQFactoryNotSet() { - // ConnectionFactory connectionFactory = new ConnectionFactory(); - // connectionFactory.useSslProtocol(); - // } -} diff --git a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.expected b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.java b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.java new file mode 100644 index 000000000000..5375c7c329ea --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.java @@ -0,0 +1,186 @@ +import java.net.Socket; +import java.nio.ByteBuffer; +import java.security.NoSuchAlgorithmException; +import javax.net.SocketFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import com.rabbitmq.client.ConnectionFactory; +import com.rabbitmq.client.SslContextFactory; + +public class UnsafeCertTrustTest { + + /** + * Test the endpoint identification of SSL engine is set to null + */ + public void testSSLEngineEndpointIdSetNull() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLEngine sslEngine = sslContext.createSSLEngine(); + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm(null); + sslEngine.setSSLParameters(sslParameters); + sslEngine.beginHandshake(); // $hasUnsafeCertTrust + sslEngine.wrap(new ByteBuffer[] {}, null); // $hasUnsafeCertTrust + sslEngine.unwrap(null, null, 0, 0); // $hasUnsafeCertTrust + } + + public void testSSLEngineEndpointIdSetEmpty() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLEngine sslEngine = sslContext.createSSLEngine(); + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm(""); + sslEngine.setSSLParameters(sslParameters); + sslEngine.beginHandshake(); // $hasUnsafeCertTrust + sslEngine.wrap(new ByteBuffer[] {}, null); // $hasUnsafeCertTrust + sslEngine.unwrap(null, null, 0, 0); // $hasUnsafeCertTrust + } + + public void testSSLEngineEndpointIdSafe() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLEngine sslEngine = sslContext.createSSLEngine(); + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + sslEngine.setSSLParameters(sslParameters); + sslEngine.beginHandshake(); // Safe + sslEngine.wrap(new ByteBuffer[] {}, null); // Safe + sslEngine.unwrap(null, null, 0, 0); // Safe + } + + public void testSSLEngineInServerMode() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLEngine sslEngine = sslContext.createSSLEngine(); + sslEngine.setUseClientMode(false); + sslEngine.beginHandshake(); // Safe + sslEngine.wrap(new ByteBuffer[] {}, null); // Safe + sslEngine.unwrap(null, null, 0, 0); // Safe + } + + public void testSSLSocketEndpointIdNotSet() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + socket.getOutputStream(); // $hasUnsafeCertTrust + } + + public void testSSLSocketEndpointIdSetNull() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm(null); + socket.setSSLParameters(sslParameters); + socket.getOutputStream(); // $hasUnsafeCertTrust + } + + public void testSSLSocketEndpointIdSetEmpty() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm(""); + socket.setSSLParameters(sslParameters); + socket.getOutputStream(); // $hasUnsafeCertTrust + } + + public void testSSLSocketEndpointIdAfterConnecting() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + socket.getOutputStream(); // $hasUnsafeCertTrust + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + socket.setSSLParameters(sslParameters); + } + + public void testSSLSocketEndpointIdSafe() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + socket.setSSLParameters(sslParameters); + socket.getOutputStream(); // Safe + } + + public void testSSLSocketEndpointIdSafeWithModificationByReference() throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + SSLParameters sslParameters = socket.getSSLParameters(); + onSetSSLParameters(sslParameters); + socket.setSSLParameters(sslParameters); + socket.getOutputStream(); // Safe + } + + private void onSetSSLParameters(SSLParameters sslParameters) { + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + } + + public void testSSLSocketEndpointIdSafeWithConditionalSanitizer(boolean safe) throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket(); + if (safe) { + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + socket.setSSLParameters(sslParameters); + } + socket.getOutputStream(); // Safe + } + + public void testSSLSocketEndpointIdSafeWithSanitizerInCast(boolean safe) throws Exception { + SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLSocketFactory socketFactory = sslContext.getSocketFactory(); + Socket socket = socketFactory.createSocket(); + SSLSocket sslSocket = (SSLSocket) socket; + SSLParameters sslParameters = sslSocket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + sslSocket.setSSLParameters(sslParameters); + socket.getOutputStream(); // $ SPURIOUS: hasUnsafeCertTrust + } + + public void testSocketEndpointIdNotSet() throws Exception { + SocketFactory socketFactory = SocketFactory.getDefault(); + Socket socket = socketFactory.createSocket("www.example.com", 80); + socket.getOutputStream(); // Safe + } + + public void testRabbitMQFactoryEnableHostnameVerificationNotSet() throws Exception { + { + ConnectionFactory connectionFactory = new ConnectionFactory(); + connectionFactory.useSslProtocol(SSLContext.getDefault()); // $hasUnsafeCertTrust + } + { + ConnectionFactory connectionFactory = new ConnectionFactory(); + connectionFactory.setSslContextFactory(new TestSslContextFactory()); // $hasUnsafeCertTrust + } + } + + public void testRabbitMQFactorySafe() throws Exception { + { + ConnectionFactory connectionFactory = new ConnectionFactory(); + connectionFactory.useSslProtocol(SSLContext.getDefault()); // Safe + connectionFactory.enableHostnameVerification(); + } + { + ConnectionFactory connectionFactory = new ConnectionFactory(); + connectionFactory.setSslContextFactory(new TestSslContextFactory()); // Safe + connectionFactory.enableHostnameVerification(); + } + } + + static class TestSslContextFactory implements SslContextFactory { + + @Override + public SSLContext create(String name) { + try { + return SSLContext.getDefault(); + } catch (NoSuchAlgorithmException e) { + return null; + } + } + + } +} diff --git a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql new file mode 100644 index 000000000000..344faa7f86b6 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql @@ -0,0 +1,24 @@ +import java +import semmle.code.java.security.UnsafeCertTrustQuery +import TestUtilities.InlineExpectationsTest + +class UnsafeCertTrustTest extends InlineExpectationsTest { + UnsafeCertTrustTest() { this = "HasUnsafeCertTrustTest" } + + override string getARelevantTag() { result = "hasUnsafeCertTrust" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasUnsafeCertTrust" and + exists(Expr unsafeTrust | + unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet + or + exists(SslEndpointIdentificationFlowConfig config | + config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) + ) + | + unsafeTrust.getLocation() = location and + element = unsafeTrust.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-273/options b/java/ql/test/query-tests/security/CWE-273/options new file mode 100644 index 000000000000..a97d4b5eeeee --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-273/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/amqp-client-5.12.0 \ No newline at end of file diff --git a/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/ConnectionFactory.java b/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/ConnectionFactory.java new file mode 100644 index 000000000000..44dd5df1a266 --- /dev/null +++ b/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/ConnectionFactory.java @@ -0,0 +1,232 @@ +// Copyright (c) 2007-2020 VMware, Inc. or its affiliates. All rights reserved. +// +// This software, the RabbitMQ Java client library, is triple-licensed under the +// Mozilla Public License 2.0 ("MPL"), the GNU General Public License version 2 +// ("GPL") and the Apache License version 2 ("ASL"). For the MPL, please see +// LICENSE-MPL-RabbitMQ. For the GPL, please see LICENSE-GPL2. For the ASL, +// please see LICENSE-APACHE2. +// +// This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY KIND, +// either express or implied. See the LICENSE file for specific language governing +// rights and limitations of this software. +// +// If you have any questions regarding licensing, please contact us at +// info@rabbitmq.com. + +package com.rabbitmq.client; + +import javax.net.SocketFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManager; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; +import java.util.*; +import java.util.concurrent.*; +import java.util.function.Predicate; +import static java.util.concurrent.TimeUnit.MINUTES; + +public class ConnectionFactory implements Cloneable { + public static final int DEFAULT_CHANNEL_RPC_TIMEOUT = (int) MINUTES.toMillis(10); + + public String getHost() { + return null; + } + + public void setHost(String host) {} + + public static int portOrDefault(int port, boolean ssl) { + return 0; + } + + public int getPort() { + return 0; + } + + public void setPort(int port) {} + + public String getUsername() { + return null; + } + + public void setUsername(String username) {} + + public String getPassword() { + return null; + } + + public void setPassword(String password) {} + + public String getVirtualHost() { + return null; + } + + public void setVirtualHost(String virtualHost) {} + + public void setUri(URI uri) + throws URISyntaxException, NoSuchAlgorithmException, KeyManagementException {} + + public void setUri(String uriString) + throws URISyntaxException, NoSuchAlgorithmException, KeyManagementException {} + + public int getRequestedChannelMax() { + return 0; + } + + public void setRequestedChannelMax(int requestedChannelMax) {} + + public int getRequestedFrameMax() { + return 0; + } + + public void setRequestedFrameMax(int requestedFrameMax) {} + + public int getRequestedHeartbeat() { + return 0; + } + + public void setConnectionTimeout(int timeout) {} + + public int getConnectionTimeout() { + return 0; + } + + public int getHandshakeTimeout() { + return 0; + } + + public void setHandshakeTimeout(int timeout) {} + + public void setShutdownTimeout(int shutdownTimeout) {} + + public int getShutdownTimeout() { + return 0; + } + + public void setRequestedHeartbeat(int requestedHeartbeat) {} + + public Map getClientProperties() { + return null; + } + + public void setClientProperties(Map clientProperties) {} + + public SocketFactory getSocketFactory() { + return null; + } + + public void setSocketFactory(SocketFactory factory) {} + + public void setSharedExecutor(ExecutorService executor) {} + + public void setShutdownExecutor(ExecutorService executor) {} + + public void setHeartbeatExecutor(ScheduledExecutorService executor) {} + + public ThreadFactory getThreadFactory() { + return null; + } + + public void setThreadFactory(ThreadFactory threadFactory) {} + + public boolean isSSL() { + return false; + } + + public void useSslProtocol() throws NoSuchAlgorithmException, KeyManagementException {} + + public void useSslProtocol(String protocol) + throws NoSuchAlgorithmException, KeyManagementException {} + + public void useSslProtocol(String protocol, TrustManager trustManager) + throws NoSuchAlgorithmException, KeyManagementException {} + + public void useSslProtocol(SSLContext context) {} + + public void enableHostnameVerification() {} + + public static String computeDefaultTlsProtocol(String[] supportedProtocols) { + return null; + } + + public boolean isAutomaticRecoveryEnabled() { + return false; + } + + public void setAutomaticRecoveryEnabled(boolean automaticRecovery) {} + + public boolean isTopologyRecoveryEnabled() { + return false; + } + + public void setTopologyRecoveryEnabled(boolean topologyRecovery) {} + + public ExecutorService getTopologyRecoveryExecutor() { + return null; + } + + public void setTopologyRecoveryExecutor(final ExecutorService topologyRecoveryExecutor) {} + + public ConnectionFactory load(String propertyFileLocation) throws IOException { + return null; + } + + public ConnectionFactory load(String propertyFileLocation, String prefix) throws IOException { + return null; + } + + public ConnectionFactory load(Properties properties) { + return null; + } + + public ConnectionFactory load(Properties properties, String prefix) { + return null; + } + + public ConnectionFactory load(Map properties) { + return null; + } + + public ConnectionFactory load(Map properties, String prefix) { + return null; + } + + public long getNetworkRecoveryInterval() { + return 0; + } + + public void setNetworkRecoveryInterval(int networkRecoveryInterval) {} + + public void setNetworkRecoveryInterval(long networkRecoveryInterval) {} + + public void useNio() {} + + public void useBlockingIo() {} + + public void setChannelRpcTimeout(int channelRpcTimeout) {} + + public int getChannelRpcTimeout() { + return 0; + } + + public void setSslContextFactory(SslContextFactory sslContextFactory) {} + + public void setChannelShouldCheckRpcResponseType(boolean channelShouldCheckRpcResponseType) {} + + public boolean isChannelShouldCheckRpcResponseType() { + return false; + } + + public void setWorkPoolTimeout(int workPoolTimeout) {} + + public int getWorkPoolTimeout() { + return 0; + } + + public static int ensureUnsignedShort(int value) { + return 0; + } + +} diff --git a/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/SslContextFactory.java b/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/SslContextFactory.java new file mode 100644 index 000000000000..ae17f15437dc --- /dev/null +++ b/java/ql/test/stubs/amqp-client-5.12.0/com/rabbitmq/client/SslContextFactory.java @@ -0,0 +1,23 @@ +// Copyright (c) 2017-2020 VMware, Inc. or its affiliates. All rights reserved. +// +// This software, the RabbitMQ Java client library, is triple-licensed under the +// Mozilla Public License 2.0 ("MPL"), the GNU General Public License version 2 +// ("GPL") and the Apache License version 2 ("ASL"). For the MPL, please see +// LICENSE-MPL-RabbitMQ. For the GPL, please see LICENSE-GPL2. For the ASL, +// please see LICENSE-APACHE2. +// +// This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY KIND, +// either express or implied. See the LICENSE file for specific language governing +// rights and limitations of this software. +// +// If you have any questions regarding licensing, please contact us at +// info@rabbitmq.com. + +package com.rabbitmq.client; + +import javax.net.ssl.SSLContext; + +public interface SslContextFactory { + SSLContext create(String name); + +}