Skip to content

Commit 6d1ba3f

Browse files
luchua-bcluchua-bc
committed
Java: CWE-273 Unsafe certificate trust
1 parent b9ecf1a commit 6d1ba3f

File tree

6 files changed

+311
-0
lines changed

6 files changed

+311
-0
lines changed

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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+
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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>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>
10+
</overview>
11+
12+
<recommendation>
13+
<p>Validate SSL certificate in SSL authentication.</p>
14+
</recommendation>
15+
16+
<example>
17+
<p>The following two examples show two ways of configuring X509 trust cert manager and hostname verifier. In the 'BAD' case,
18+
no validation is performed thus any certificate is trusted. In the 'GOOD' case, the proper validation is performed.</p>
19+
<sample src="UnsafeCertTrust.java" />
20+
</example>
21+
22+
<references>
23+
<li>
24+
<a href="https://cwe.mitre.org/data/definitions/273.html">CWE-273</a>
25+
</li>
26+
<li>
27+
<a href="https://support.google.com/faqs/answer/6346016?hl=en">How to fix apps containing an unsafe implementation of TrustManager</a>
28+
</li>
29+
<li>
30+
<a href="https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05g-Testing-Network-Communication.md">Testing Endpoint Identify Verification (MSTG-NETWORK-3)</a>
31+
</li>
32+
</references>
33+
</qhelp>

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @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.
5+
* @kind problem
6+
* @tags security
7+
* external/cwe-273
8+
*/
9+
10+
import java
11+
import semmle.code.java.security.Encryption
12+
import semmle.code.java.dataflow.DataFlow
13+
import DataFlow
14+
15+
/**
16+
* X509TrustManager class that blindly trusts all certificates in server SSL authentication
17+
*/
18+
class X509TrustAllManager extends RefType {
19+
X509TrustAllManager() {
20+
this.getASupertype*() instanceof X509TrustManager and
21+
exists(Method m1 |
22+
m1.getDeclaringType() = this and
23+
m1.hasName("checkServerTrusted") and
24+
m1.getBody().getNumStmt() = 0
25+
) and
26+
exists(Method m2, ReturnStmt rt2 |
27+
m2.getDeclaringType() = this and
28+
m2.hasName("getAcceptedIssuers") and
29+
rt2.getEnclosingCallable() = m2 and
30+
rt2.getResult() instanceof NullLiteral
31+
)
32+
}
33+
}
34+
35+
/**
36+
* The init method of SSLContext with the trust all manager, which is sslContext.init(..., serverTMs, ...)
37+
*/
38+
class X509TrustAllManagerInit extends MethodAccess {
39+
X509TrustAllManagerInit() {
40+
this.getMethod().hasName("init") and
41+
this.getMethod().getDeclaringType() instanceof SSLContext and //init method of SSLContext
42+
(
43+
exists(ArrayInit ai |
44+
ai.getInit(0).(VarAccess).getVariable().getInitializer().getType().(Class).getASupertype*()
45+
instanceof X509TrustAllManager //Scenario of context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
46+
)
47+
or
48+
exists(Variable v, ArrayInit ai |
49+
this.getArgument(1).(VarAccess).getVariable() = v and
50+
ai.getParent() = v.getAnAccess().getVariable().getAnAssignedValue() and
51+
ai.getInit(0).getType().(Class).getASupertype*() instanceof X509TrustAllManager //Scenario of context.init(null, serverTMs, null);
52+
)
53+
)
54+
}
55+
}
56+
57+
/**
58+
* HostnameVerifier class that allows a certificate whose CN (Common Name) does not match the host name in the URL
59+
*/
60+
class TrustAllHostnameVerifier extends RefType {
61+
TrustAllHostnameVerifier() {
62+
this.getASupertype*() instanceof HostnameVerifier and
63+
exists(Method m, ReturnStmt rt |
64+
m.getDeclaringType() = this and
65+
m.hasName("verify") and
66+
rt.getEnclosingCallable() = m and
67+
rt.getResult().(BooleanLiteral).getBooleanValue() = true
68+
)
69+
}
70+
}
71+
72+
/**
73+
* The setDefaultHostnameVerifier method of HttpsURLConnection with the trust all configuration
74+
*/
75+
class TrustAllHostnameVerify extends MethodAccess {
76+
TrustAllHostnameVerify() {
77+
this.getMethod().hasName("setDefaultHostnameVerifier") and
78+
this.getMethod().getDeclaringType() instanceof HttpsURLConnection and //httpsURLConnection.setDefaultHostnameVerifier method
79+
(
80+
exists(NestedClass nc |
81+
nc.getASupertype*() instanceof TrustAllHostnameVerifier and
82+
this.getArgument(0).getType() = nc //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {...});
83+
)
84+
or
85+
exists(Variable v |
86+
this.getArgument(0).(VarAccess).getVariable() = v.getAnAccess().getVariable() and
87+
v.getInitializer().getType() instanceof TrustAllHostnameVerifier //Scenario of HttpsURLConnection.setDefaultHostnameVerifier(verifier);
88+
)
89+
)
90+
}
91+
}
92+
93+
from MethodAccess aa
94+
where aa instanceof TrustAllHostnameVerify or aa instanceof X509TrustAllManagerInit
95+
select aa, "Unsafe configuration of trusted certificates"

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
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 |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-273/UnsafeCertTrust.ql

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import javax.net.ssl.HostnameVerifier;
2+
import javax.net.ssl.HttpsURLConnection;
3+
import javax.net.ssl.SSLSession;
4+
import javax.net.ssl.SSLContext;
5+
import javax.net.ssl.SSLSocketFactory;
6+
import javax.net.ssl.TrustManager;
7+
import javax.net.ssl.X509TrustManager;
8+
import java.security.cert.CertificateException;
9+
import java.security.cert.X509Certificate;
10+
11+
public class UnsafeCertTrustTest {
12+
13+
/**
14+
* Test the implementation of trusting all server certs as a variable
15+
*/
16+
public SSLSocketFactory testTrustAllCertManager() {
17+
try {
18+
final SSLContext context = SSLContext.getInstance("SSL");
19+
context.init(null, new TrustManager[] { TRUST_ALL_CERTIFICATES }, null);
20+
final SSLSocketFactory socketFactory = context.getSocketFactory();
21+
return socketFactory;
22+
} catch (final Exception x) {
23+
throw new RuntimeException(x);
24+
}
25+
}
26+
27+
/**
28+
* Test the implementation of trusting all server certs as an anonymous class
29+
*/
30+
public SSLSocketFactory testTrustAllCertManagerOfVariable() {
31+
try {
32+
SSLContext context = SSLContext.getInstance("SSL");
33+
TrustManager[] serverTMs = new TrustManager[] { new X509TrustAllManager() };
34+
context.init(null, serverTMs, null);
35+
36+
final SSLSocketFactory socketFactory = context.getSocketFactory();
37+
return socketFactory;
38+
} catch (final Exception x) {
39+
throw new RuntimeException(x);
40+
}
41+
}
42+
43+
/**
44+
* Test the implementation of trusting all hostnames as an anonymous class
45+
*/
46+
public void testTrustAllHostnameOfAnonymousClass() {
47+
HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() {
48+
@Override
49+
public boolean verify(String hostname, SSLSession session) {
50+
return true; // Noncompliant
51+
}
52+
});
53+
}
54+
55+
/**
56+
* Test the implementation of trusting all hostnames as a variable
57+
*/
58+
public void testTrustAllHostnameOfVariable() {
59+
HostnameVerifier verifier = new HostnameVerifier() {
60+
@Override
61+
public boolean verify(String hostname, SSLSession session) {
62+
return true; // Noncompliant
63+
}
64+
};
65+
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
66+
}
67+
68+
private static final X509TrustManager TRUST_ALL_CERTIFICATES = new X509TrustManager() {
69+
@Override
70+
public void checkClientTrusted(final X509Certificate[] chain, final String authType)
71+
throws CertificateException {
72+
}
73+
74+
@Override
75+
public void checkServerTrusted(final X509Certificate[] chain, final String authType)
76+
throws CertificateException {
77+
// Noncompliant
78+
}
79+
80+
@Override
81+
public X509Certificate[] getAcceptedIssuers() {
82+
return null; // Noncompliant
83+
}
84+
};
85+
86+
private class X509TrustAllManager implements X509TrustManager {
87+
@Override
88+
public void checkClientTrusted(final X509Certificate[] chain, final String authType)
89+
throws CertificateException {
90+
}
91+
92+
@Override
93+
public void checkServerTrusted(final X509Certificate[] chain, final String authType)
94+
throws CertificateException {
95+
// Noncompliant
96+
}
97+
98+
@Override
99+
public X509Certificate[] getAcceptedIssuers() {
100+
return null; // Noncompliant
101+
}
102+
};
103+
104+
public static final HostnameVerifier ALLOW_ALL_HOSTNAME_VERIFIER = new HostnameVerifier() {
105+
@Override
106+
public boolean verify(String hostname, SSLSession session) {
107+
return true; // Noncompliant
108+
}
109+
};
110+
}

0 commit comments

Comments
 (0)