Skip to content

Commit 104f1c3

Browse files
luchua-bcluchua-bc
committed
Add validation query for SSL Engine/Socket and com.rabbitmq.client.ConnectionFactory
1 parent 6d1ba3f commit 104f1c3

File tree

5 files changed

+241
-16
lines changed

5 files changed

+241
-16
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,37 @@ public X509Certificate[] getAcceptedIssuers() {
6565
}
6666
};
6767
}
68+
69+
{
70+
SSLContext sslContext = SSLContext.getInstance("TLS");
71+
SSLEngine sslEngine = sslContext.createSSLEngine();
72+
SSLParameters sslParameters = sslEngine.getSSLParameters();
73+
sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); //GOOD: Set a valid endpointIdentificationAlgorithm for SSL engine to trigger hostname verification
74+
sslEngine.setSSLParameters(sslParameters);
75+
}
76+
77+
{
78+
SSLContext sslContext = SSLContext.getInstance("TLS");
79+
SSLEngine sslEngine = sslContext.createSSLEngine(); //BAD: No endpointIdentificationAlgorithm set
80+
}
81+
82+
{
83+
SSLContext sslContext = SSLContext.getInstance("TLS");
84+
final SSLSocketFactory socketFactory = sslContext.getSocketFactory();
85+
SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
86+
SSLParameters sslParameters = sslEngine.getSSLParameters();
87+
sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); //GOOD: Set a valid endpointIdentificationAlgorithm for SSL socket to trigger hostname verification
88+
socket.setSSLParameters(sslParameters);
89+
}
90+
91+
{
92+
com.rabbitmq.client.ConnectionFactory connectionFactory = new com.rabbitmq.client.ConnectionFactory();
93+
connectionFactory.useSslProtocol();
94+
connectionFactory.enableHostnameVerification(); //GOOD: Enable hostname verification for rabbitmq ConnectionFactory
95+
}
96+
97+
{
98+
com.rabbitmq.client.ConnectionFactory connectionFactory = new com.rabbitmq.client.ConnectionFactory();
99+
connectionFactory.useSslProtocol(); //BAD: Hostname verification for rabbitmq ConnectionFactory is not enabled
100+
}
68101
}

‎java/ql/src/experimental/Security/CWE/CWE-273/UnsafeCertTrust.qhelp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55

66
<overview>
77
<p>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.</p>
8-
<p>Unsafe implementation of the interface X509TrustManager and HostnameVerifier ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.</p>
9-
<p>This query checks whether trust manager is set to trust all certificates or the hostname verifier is turned off.</p>
8+
<p>And when SSLSocket or SSLEngine is created without a valid parameter of setEndpointIdentificationAlgorithm, hostname verification is disabled by default.</p>
9+
<p>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.</p>
10+
<p>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.</p>
1011
</overview>
1112

1213
<recommendation>
@@ -29,5 +30,17 @@ no validation is performed thus any certificate is trusted. In the 'GOOD' case,
2930
<li>
3031
<a href="https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05g-Testing-Network-Communication.md">Testing Endpoint Identify Verification (MSTG-NETWORK-3)</a>
3132
</li>
33+
<li>
34+
<a href="https://github.com/advisories/GHSA-xvch-r4wf-h8w9">CVE-2018-17187: Apache Qpid Proton-J transport issue with hostname verification</a>
35+
</li>
36+
<li>
37+
<a href="https://github.com/advisories/GHSA-46j3-r4pj-4835">CVE-2018-8034: Apache Tomcat - host name verification when using TLS with the WebSocket client</a>
38+
</li>
39+
<li>
40+
<a href="https://github.com/advisories/GHSA-w4g2-9hj6-5472">CVE-2018-11087: Pivotal Spring AMQP vulnerability due to lack of hostname validation</a>
41+
</li>
42+
<li>
43+
<a href="https://github.com/advisories/GHSA-m9w8-v359-9ffr">CVE-2018-11775: TLS hostname verification issue when using the Apache ActiveMQ Client</a>
44+
</li>
3245
</references>
33-
</qhelp>
46+
</qhelp>

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

Lines changed: 139 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
/**
22
* @id java/unsafe-cert-trust
3-
* @name Unsafe implementation of trusting any certificate in SSL configuration
4-
* @description Unsafe implementation of the interface X509TrustManager and HostnameVerifier ignores all SSL certificate validation errors when establishing an HTTPS connection, thereby making the app vulnerable to man-in-the-middle attacks.
3+
* @name Unsafe implementation of trusting any certificate or missing hostname verification in SSL configuration
4+
* @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.
55
* @kind problem
66
* @tags security
77
* external/cwe-273
88
*/
99

1010
import java
1111
import semmle.code.java.security.Encryption
12-
import semmle.code.java.dataflow.DataFlow
13-
import DataFlow
1412

1513
/**
1614
* X509TrustManager class that blindly trusts all certificates in server SSL authentication
@@ -79,7 +77,7 @@ class TrustAllHostnameVerify extends MethodAccess {
7977
(
8078
exists(NestedClass nc |
8179
nc.getASupertype*() instanceof TrustAllHostnameVerifier and
82-
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
80+
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
8381
)
8482
or
8583
exists(Variable v |
@@ -90,6 +88,141 @@ class TrustAllHostnameVerify extends MethodAccess {
9088
}
9189
}
9290

91+
class SSLEngine extends RefType {
92+
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
93+
}
94+
95+
class Socket extends RefType {
96+
Socket() { this.hasQualifiedName("java.net", "Socket") }
97+
}
98+
99+
class SSLSocket extends RefType {
100+
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
101+
}
102+
103+
/**
104+
* has setEndpointIdentificationAlgorithm set correctly
105+
*/
106+
predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) {
107+
exists(
108+
Variable sslo, MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
109+
|
110+
createSSL = sslo.getAnAssignedValue() and
111+
ma.getQualifier() = sslo.getAnAccess() and
112+
ma.getMethod().hasName("setSSLParameters") and
113+
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
114+
exists(MethodAccess setepa |
115+
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
116+
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
117+
not setepa.getArgument(0) instanceof NullLiteral
118+
)
119+
)
120+
}
121+
122+
/**
123+
* has setEndpointIdentificationAlgorithm set correctly
124+
*/
125+
predicate hasEndpointIdentificationAlgorithm(Variable ssl) {
126+
exists(
127+
MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
128+
|
129+
ma.getQualifier() = ssl.getAnAccess() and
130+
ma.getMethod().hasName("setSSLParameters") and
131+
ma.getArgument(0).(VarAccess) = sslparams.getAnAccess() and
132+
exists(MethodAccess setepa |
133+
setepa.getQualifier().(VarAccess) = sslparams.getAnAccess() and
134+
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
135+
not setepa.getArgument(0) instanceof NullLiteral
136+
)
137+
)
138+
}
139+
140+
/**
141+
* SSL object is created in a separate method call or in the same method
142+
*/
143+
predicate hasFlowPath(MethodAccess createSSL, Variable ssl) {
144+
(
145+
createSSL = ssl.getAnAssignedValue()
146+
or
147+
exists(CastExpr ce |
148+
ce.getExpr().(MethodAccess) = createSSL and
149+
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
150+
)
151+
)
152+
or
153+
exists(MethodAccess tranm |
154+
createSSL.getEnclosingCallable().(Method) = tranm.getMethod() and
155+
tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
156+
not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method
157+
)
158+
}
159+
160+
/**
161+
* Not have the SSLParameter set
162+
*/
163+
predicate hasNoEndpointIdentificationSet(MethodAccess createSSL, Variable ssl) {
164+
//No setSSLParameters set
165+
hasFlowPath(createSSL, ssl) and
166+
not exists(MethodAccess ma |
167+
ma.getQualifier() = ssl.getAnAccess() and
168+
ma.getMethod().hasName("setSSLParameters")
169+
)
170+
or
171+
//No endpointIdentificationAlgorithm set with setSSLParameters
172+
hasFlowPath(createSSL, ssl) and
173+
not setEndpointIdentificationAlgorithm(createSSL)
174+
}
175+
176+
/**
177+
* The setEndpointIdentificationAlgorithm method of SSLParameters with the ssl engine or socket
178+
*/
179+
class SSLEndpointIdentificationNotSet extends MethodAccess {
180+
SSLEndpointIdentificationNotSet() {
181+
(
182+
this.getMethod().hasName("createSSLEngine") and
183+
this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext
184+
or
185+
this.getMethod().hasName("createSocket") and
186+
this.getMethod().getReturnType() instanceof Socket //createSocket method of SSLSocketFactory
187+
) and
188+
exists(Variable ssl |
189+
hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself
190+
not exists(VariableAssign ar, Variable newSsl |
191+
ar.getSource() = this.getCaller().getAReference() and
192+
ar.getDestVar() = newSsl and
193+
hasEndpointIdentificationAlgorithm(newSsl) //Not set in its caller either
194+
)
195+
) and
196+
not exists(MethodAccess ma | ma.getMethod() instanceof HostnameVerifierVerify) //Reduce false positives since this method access set default hostname verifier
197+
}
198+
}
199+
200+
class RabbitMQConnectionFactory extends RefType {
201+
RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") }
202+
}
203+
204+
/**
205+
* The com.rabbitmq.client.ConnectionFactory useSslProtocol method access without enableHostnameVerification
206+
*/
207+
class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
208+
RabbitMQEnableHostnameVerificationNotSet() {
209+
this.getMethod().hasName("useSslProtocol") and
210+
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
211+
exists(VarAccess va |
212+
va.getVariable().getType() instanceof RabbitMQConnectionFactory and
213+
this.getQualifier() = va.getVariable().getAnAccess() and
214+
not exists(MethodAccess ma |
215+
ma.getMethod().hasName("enableHostnameVerification") and
216+
ma.getQualifier() = va.getVariable().getAnAccess()
217+
)
218+
)
219+
}
220+
}
221+
93222
from MethodAccess aa
94-
where aa instanceof TrustAllHostnameVerify or aa instanceof X509TrustAllManagerInit
223+
where
224+
aa instanceof TrustAllHostnameVerify or
225+
aa instanceof X509TrustAllManagerInit or
226+
aa instanceof SSLEndpointIdentificationNotSet or
227+
aa instanceof RabbitMQEnableHostnameVerificationNotSet
95228
select aa, "Unsafe configuration of trusted certificates"

‎java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrust.expected

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
| UnsafeCertTrustTest.java:19:4:19:74 | init(...) | Unsafe configuration of trusted certificates |
2-
| UnsafeCertTrustTest.java:34:4:34:38 | init(...) | Unsafe configuration of trusted certificates |
3-
| UnsafeCertTrustTest.java:47:3:52:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
4-
| UnsafeCertTrustTest.java:65:3:65:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
1+
| UnsafeCertTrustTest.java:26:4:26:74 | init(...) | Unsafe configuration of trusted certificates |
2+
| UnsafeCertTrustTest.java:41:4:41:38 | init(...) | Unsafe configuration of trusted certificates |
3+
| UnsafeCertTrustTest.java:54:3:59:4 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
4+
| UnsafeCertTrustTest.java:72:3:72:57 | setDefaultHostnameVerifier(...) | Unsafe configuration of trusted certificates |
5+
| UnsafeCertTrustTest.java:123:25:123:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
6+
| UnsafeCertTrustTest.java:134:25:134:52 | createSSLEngine(...) | Unsafe configuration of trusted certificates |
7+
| UnsafeCertTrustTest.java:143:34:143:83 | createSocket(...) | Unsafe configuration of trusted certificates |

‎java/ql/test/experimental/query-tests/security/CWE-273/UnsafeCertTrustTest.java

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,28 @@
11
import javax.net.ssl.HostnameVerifier;
22
import javax.net.ssl.HttpsURLConnection;
33
import javax.net.ssl.SSLSession;
4+
import javax.net.ssl.SSLSocket;
45
import javax.net.ssl.SSLContext;
6+
import javax.net.ssl.SSLEngine;
7+
import javax.net.ssl.SSLParameters;
58
import javax.net.ssl.SSLSocketFactory;
69
import javax.net.ssl.TrustManager;
710
import javax.net.ssl.X509TrustManager;
11+
12+
import java.net.Socket;
813
import java.security.cert.CertificateException;
914
import java.security.cert.X509Certificate;
1015

16+
//import com.rabbitmq.client.ConnectionFactory;
17+
1118
public class UnsafeCertTrustTest {
1219

1320
/**
1421
* Test the implementation of trusting all server certs as a variable
1522
*/
1623
public SSLSocketFactory testTrustAllCertManager() {
1724
try {
18-
final SSLContext context = SSLContext.getInstance("SSL");
25+
final SSLContext context = SSLContext.getInstance("TLS");
1926
context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
2027
final SSLSocketFactory socketFactory = context.getSocketFactory();
2128
return socketFactory;
@@ -29,7 +36,7 @@ public SSLSocketFactory testTrustAllCertManager() {
2936
*/
3037
public SSLSocketFactory testTrustAllCertManagerOfVariable() {
3138
try {
32-
SSLContext context = SSLContext.getInstance("SSL");
39+
SSLContext context = SSLContext.getInstance("TLS");
3340
TrustManager[] serverTMs = new TrustManager[] { new X509TrustAllManager() };
3441
context.init(null, serverTMs, null);
3542

@@ -107,4 +114,40 @@ public boolean verify(String hostname, SSLSession session) {
107114
return true; // Noncompliant
108115
}
109116
};
110-
}
117+
118+
/**
119+
* Test the endpoint identification of SSL engine is set to null
120+
*/
121+
public void testSSLEngineEndpointIdSetNull() {
122+
SSLContext sslContext = SSLContext.getInstance("TLS");
123+
SSLEngine sslEngine = sslContext.createSSLEngine();
124+
SSLParameters sslParameters = sslEngine.getSSLParameters();
125+
sslParameters.setEndpointIdentificationAlgorithm(null);
126+
sslEngine.setSSLParameters(sslParameters);
127+
}
128+
129+
/**
130+
* Test the endpoint identification of SSL engine is not set
131+
*/
132+
public void testSSLEngineEndpointIdNotSet() {
133+
SSLContext sslContext = SSLContext.getInstance("TLS");
134+
SSLEngine sslEngine = sslContext.createSSLEngine();
135+
}
136+
137+
/**
138+
* Test the endpoint identification of SSL socket is not set
139+
*/
140+
public void testSSLSocketEndpointIdNotSet() {
141+
SSLContext sslContext = SSLContext.getInstance("TLS");
142+
final SSLSocketFactory socketFactory = sslContext.getSocketFactory();
143+
SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
144+
}
145+
146+
// /**
147+
// * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
148+
// */
149+
// public void testEnableHostnameVerificationOfRabbitMQFactoryNotSet() {
150+
// ConnectionFactory connectionFactory = new ConnectionFactory();
151+
// connectionFactory.useSslProtocol();
152+
// }
153+
}

0 commit comments

Comments
 (0)