Skip to content

Commit b53b905

Browse files
authored
Merge pull request github#3550 from luchua-bc/java-unsafe-cert-trust
Java: CWE-273 Unsafe certificate trust
2 parents da8725a + 0779aab commit b53b905

File tree

6 files changed

+563
-0
lines changed

6 files changed

+563
-0
lines changed
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
public static void main(String[] args) {
2+
{
3+
HostnameVerifier verifier = new HostnameVerifier() {
4+
@Override
5+
public boolean verify(String hostname, SSLSession session) {
6+
try { //GOOD: verify the certificate
7+
Certificate[] certs = session.getPeerCertificates();
8+
X509Certificate x509 = (X509Certificate) certs[0];
9+
check(new String[]{host}, x509);
10+
return true;
11+
} catch (SSLException e) {
12+
return false;
13+
}
14+
}
15+
};
16+
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
17+
}
18+
19+
{
20+
HostnameVerifier verifier = new HostnameVerifier() {
21+
@Override
22+
public boolean verify(String hostname, SSLSession session) {
23+
return true; // BAD: accept even if the hostname doesn't match
24+
}
25+
};
26+
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
27+
}
28+
29+
{
30+
X509TrustManager trustAllCertManager = new X509TrustManager() {
31+
@Override
32+
public void checkClientTrusted(final X509Certificate[] chain, final String authType)
33+
throws CertificateException {
34+
}
35+
36+
@Override
37+
public void checkServerTrusted(final X509Certificate[] chain, final String authType)
38+
throws CertificateException {
39+
// BAD: trust any server cert
40+
}
41+
42+
@Override
43+
public X509Certificate[] getAcceptedIssuers() {
44+
return null; //BAD: doesn't check cert issuer
45+
}
46+
};
47+
}
48+
49+
{
50+
X509TrustManager trustCertManager = new X509TrustManager() {
51+
@Override
52+
public void checkClientTrusted(final X509Certificate[] chain, final String authType)
53+
throws CertificateException {
54+
}
55+
56+
@Override
57+
public void checkServerTrusted(final X509Certificate[] chain, final String authType)
58+
throws CertificateException {
59+
pkixTrustManager.checkServerTrusted(chain, authType); //GOOD: validate the server cert
60+
}
61+
62+
@Override
63+
public X509Certificate[] getAcceptedIssuers() {
64+
return new X509Certificate[0]; //GOOD: Validate the cert issuer
65+
}
66+
};
67+
}
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+
}
101+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<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>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>
11+
</overview>
12+
13+
<recommendation>
14+
<p>Validate SSL certificate in SSL authentication.</p>
15+
</recommendation>
16+
17+
<example>
18+
<p>The following two examples show two ways of configuring X509 trust cert manager and hostname verifier. In the 'BAD' case,
19+
no validation is performed thus any certificate is trusted. In the 'GOOD' case, the proper validation is performed.</p>
20+
<sample src="UnsafeCertTrust.java" />
21+
</example>
22+
23+
<references>
24+
<li>
25+
<a href="https://cwe.mitre.org/data/definitions/273.html">CWE-273</a>
26+
</li>
27+
<li>
28+
<a href="https://support.google.com/faqs/answer/6346016?hl=en">How to fix apps containing an unsafe implementation of TrustManager</a>
29+
</li>
30+
<li>
31+
<a href="https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05g-Testing-Network-Communication.md">Testing Endpoint Identify Verification (MSTG-NETWORK-3)</a>
32+
</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>
45+
</references>
46+
</qhelp>
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
/**
2+
* @name Unsafe certificate trust and improper hostname verification
3+
* @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.
4+
* @kind problem
5+
* @id java/unsafe-cert-trust
6+
* @tags security
7+
* external/cwe-273
8+
*/
9+
10+
import java
11+
import semmle.code.java.security.Encryption
12+
13+
/**
14+
* X509TrustManager class that blindly trusts all certificates in server SSL authentication
15+
*/
16+
class X509TrustAllManager extends RefType {
17+
X509TrustAllManager() {
18+
this.getASupertype*() instanceof X509TrustManager and
19+
exists(Method m1 |
20+
m1.getDeclaringType() = this and
21+
m1.hasName("checkServerTrusted") and
22+
m1.getBody().getNumStmt() = 0
23+
) and
24+
exists(Method m2, ReturnStmt rt2 |
25+
m2.getDeclaringType() = this and
26+
m2.hasName("getAcceptedIssuers") and
27+
rt2.getEnclosingCallable() = m2 and
28+
rt2.getResult() instanceof NullLiteral
29+
)
30+
}
31+
}
32+
33+
/**
34+
* The init method of SSLContext with the trust all manager, which is sslContext.init(..., serverTMs, ...)
35+
*/
36+
class X509TrustAllManagerInit extends MethodAccess {
37+
X509TrustAllManagerInit() {
38+
this.getMethod().hasName("init") and
39+
this.getMethod().getDeclaringType() instanceof SSLContext and //init method of SSLContext
40+
(
41+
exists(ArrayInit ai |
42+
this.getArgument(1).(ArrayCreationExpr).getInit() = ai and
43+
ai.getInit(0).(VarAccess).getVariable().getInitializer().getType().(Class).getASupertype*()
44+
instanceof X509TrustAllManager //Scenario of context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
45+
)
46+
or
47+
exists(Variable v, ArrayInit ai |
48+
this.getArgument(1).(VarAccess).getVariable() = v and
49+
ai.getParent() = v.getAnAssignedValue() and
50+
ai.getInit(0).getType().(Class).getASupertype*() instanceof X509TrustAllManager //Scenario of context.init(null, serverTMs, null);
51+
)
52+
)
53+
}
54+
}
55+
56+
/**
57+
* HostnameVerifier class that allows a certificate whose CN (Common Name) does not match the host name in the URL
58+
*/
59+
class TrustAllHostnameVerifier extends RefType {
60+
TrustAllHostnameVerifier() {
61+
this.getASupertype*() instanceof HostnameVerifier and
62+
exists(Method m, ReturnStmt rt |
63+
m.getDeclaringType() = this and
64+
m.hasName("verify") and
65+
rt.getEnclosingCallable() = m and
66+
rt.getResult().(BooleanLiteral).getBooleanValue() = true
67+
)
68+
}
69+
}
70+
71+
/**
72+
* The setDefaultHostnameVerifier method of HttpsURLConnection with the trust all configuration
73+
*/
74+
class TrustAllHostnameVerify extends MethodAccess {
75+
TrustAllHostnameVerify() {
76+
this.getMethod().hasName("setDefaultHostnameVerifier") and
77+
this.getMethod().getDeclaringType() instanceof HttpsURLConnection and //httpsURLConnection.setDefaultHostnameVerifier method
78+
(
79+
exists(NestedClass nc |
80+
nc.getASupertype*() instanceof TrustAllHostnameVerifier and
81+
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
82+
)
83+
or
84+
exists(Variable v |
85+
this.getArgument(0).(VarAccess).getVariable() = v and
86+
v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier);
87+
)
88+
)
89+
}
90+
}
91+
92+
class SSLEngine extends RefType {
93+
SSLEngine() { this.hasQualifiedName("javax.net.ssl", "SSLEngine") }
94+
}
95+
96+
class Socket extends RefType {
97+
Socket() { this.hasQualifiedName("java.net", "Socket") }
98+
}
99+
100+
class SocketFactory extends RefType {
101+
SocketFactory() { this.hasQualifiedName("javax.net", "SocketFactory") }
102+
}
103+
104+
class SSLSocket extends RefType {
105+
SSLSocket() { this.hasQualifiedName("javax.net.ssl", "SSLSocket") }
106+
}
107+
108+
/**
109+
* has setEndpointIdentificationAlgorithm set correctly
110+
*/
111+
predicate setEndpointIdentificationAlgorithm(MethodAccess createSSL) {
112+
exists(
113+
Variable sslo, MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
114+
|
115+
createSSL = sslo.getAnAssignedValue() and
116+
ma.getQualifier() = sslo.getAnAccess() and
117+
ma.getMethod().hasName("setSSLParameters") and
118+
ma.getArgument(0) = sslparams.getAnAccess() and
119+
exists(MethodAccess setepa |
120+
setepa.getQualifier() = sslparams.getAnAccess() and
121+
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
122+
not setepa.getArgument(0) instanceof NullLiteral
123+
)
124+
)
125+
}
126+
127+
/**
128+
* has setEndpointIdentificationAlgorithm set correctly
129+
*/
130+
predicate hasEndpointIdentificationAlgorithm(Variable ssl) {
131+
exists(
132+
MethodAccess ma, Variable sslparams //setSSLParameters with valid setEndpointIdentificationAlgorithm set
133+
|
134+
ma.getQualifier() = ssl.getAnAccess() and
135+
ma.getMethod().hasName("setSSLParameters") and
136+
ma.getArgument(0) = sslparams.getAnAccess() and
137+
exists(MethodAccess setepa |
138+
setepa.getQualifier() = sslparams.getAnAccess() and
139+
setepa.getMethod().hasName("setEndpointIdentificationAlgorithm") and
140+
not setepa.getArgument(0) instanceof NullLiteral
141+
)
142+
)
143+
}
144+
145+
/**
146+
* Cast of Socket to SSLSocket
147+
*/
148+
predicate sslCast(MethodAccess createSSL) {
149+
exists(Variable ssl, CastExpr ce |
150+
ce.getExpr() = createSSL and
151+
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
152+
ssl.getType() instanceof SSLSocket //With a type cast `SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443)`
153+
)
154+
}
155+
156+
/**
157+
* SSL object is created in a separate method call or in the same method
158+
*/
159+
predicate hasFlowPath(MethodAccess createSSL, Variable ssl) {
160+
(
161+
createSSL = ssl.getAnAssignedValue()
162+
or
163+
exists(CastExpr ce |
164+
ce.getExpr() = createSSL and
165+
ce.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl //With a type cast like SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443);
166+
)
167+
)
168+
or
169+
exists(MethodAccess tranm |
170+
createSSL.getEnclosingCallable() = tranm.getMethod() and
171+
tranm.getControlFlowNode().getASuccessor().(VariableAssign).getDestVar() = ssl and
172+
not setEndpointIdentificationAlgorithm(createSSL) //Check the scenario of invocation before used in the current method
173+
)
174+
}
175+
176+
/**
177+
* Not have the SSLParameter set
178+
*/
179+
predicate hasNoEndpointIdentificationSet(MethodAccess createSSL, Variable ssl) {
180+
//No setSSLParameters set
181+
hasFlowPath(createSSL, ssl) and
182+
not exists(MethodAccess ma |
183+
ma.getQualifier() = ssl.getAnAccess() and
184+
ma.getMethod().hasName("setSSLParameters")
185+
)
186+
or
187+
//No endpointIdentificationAlgorithm set with setSSLParameters
188+
hasFlowPath(createSSL, ssl) and
189+
not setEndpointIdentificationAlgorithm(createSSL)
190+
}
191+
192+
/**
193+
* The setEndpointIdentificationAlgorithm method of SSLParameters with the ssl engine or socket
194+
*/
195+
class SSLEndpointIdentificationNotSet extends MethodAccess {
196+
SSLEndpointIdentificationNotSet() {
197+
(
198+
this.getMethod().hasName("createSSLEngine") and
199+
this.getMethod().getDeclaringType() instanceof SSLContext //createEngine method of SSLContext
200+
or
201+
this.getMethod().hasName("createSocket") and
202+
this.getMethod().getDeclaringType() instanceof SocketFactory and
203+
this.getMethod().getReturnType() instanceof Socket and
204+
sslCast(this) //createSocket method of SocketFactory
205+
) and
206+
exists(Variable ssl |
207+
hasNoEndpointIdentificationSet(this, ssl) and //Not set in itself
208+
not exists(VariableAssign ar, Variable newSsl |
209+
ar.getSource() = this.getCaller().getAReference() and
210+
ar.getDestVar() = newSsl and
211+
hasEndpointIdentificationAlgorithm(newSsl) //Not set in its caller either
212+
)
213+
) and
214+
not exists(MethodAccess ma | ma.getMethod() instanceof HostnameVerifierVerify) //Reduce false positives since this method access set default hostname verifier
215+
}
216+
}
217+
218+
class RabbitMQConnectionFactory extends RefType {
219+
RabbitMQConnectionFactory() { this.hasQualifiedName("com.rabbitmq.client", "ConnectionFactory") }
220+
}
221+
222+
/**
223+
* The com.rabbitmq.client.ConnectionFactory useSslProtocol method access without enableHostnameVerification
224+
*/
225+
class RabbitMQEnableHostnameVerificationNotSet extends MethodAccess {
226+
RabbitMQEnableHostnameVerificationNotSet() {
227+
this.getMethod().hasName("useSslProtocol") and
228+
this.getMethod().getDeclaringType() instanceof RabbitMQConnectionFactory and
229+
exists(Variable v |
230+
v.getType() instanceof RabbitMQConnectionFactory and
231+
this.getQualifier() = v.getAnAccess() and
232+
not exists(MethodAccess ma |
233+
ma.getMethod().hasName("enableHostnameVerification") and
234+
ma.getQualifier() = v.getAnAccess()
235+
)
236+
)
237+
}
238+
}
239+
240+
from MethodAccess aa
241+
where
242+
aa instanceof TrustAllHostnameVerify or
243+
aa instanceof X509TrustAllManagerInit or
244+
aa instanceof SSLEndpointIdentificationNotSet or
245+
aa instanceof RabbitMQEnableHostnameVerificationNotSet
246+
select aa, "Unsafe configuration of trusted certificates"

0 commit comments

Comments
 (0)