Skip to content

Commit 87554a7

Browse files
intrigusintrigus
committed
Java: Add insecure trust manager query.
1 parent 2d24387 commit 87554a7

File tree

6 files changed

+379
-0
lines changed

6 files changed

+379
-0
lines changed

‎java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
public static void main(String[] args) throws Exception {
2+
{
3+
class InsecureTrustManager implements X509TrustManager {
4+
@Override
5+
public X509Certificate[] getAcceptedIssuers() {
6+
return null;
7+
}
8+
9+
@Override
10+
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
11+
// BAD: Does not verify the certificate chain, allowing any certificate.
12+
}
13+
14+
@Override
15+
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
16+
17+
}
18+
}
19+
SSLContext context = SSLContext.getInstance("TLS");
20+
TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
21+
context.init(null, trustManager, null);
22+
}
23+
{
24+
SSLContext context = SSLContext.getInstance("TLS");
25+
File certificateFile = new File("path/to/self-signed-certificate");
26+
// Create a `KeyStore` with default type
27+
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
28+
// This causes `keyStore` to be empty
29+
keyStore.load(null, null);
30+
X509Certificate generatedCertificate;
31+
try (InputStream cert = new FileInputStream(certificateFile)) {
32+
generatedCertificate = (X509Certificate) CertificateFactory.getInstance("X509")
33+
.generateCertificate(cert);
34+
}
35+
// Add the self-signed certificate to the key store
36+
keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate);
37+
// Get default `TrustManagerFactory`
38+
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
39+
// Use it with our modified key store that trusts our self-signed certificate
40+
tmf.init(keyStore);
41+
TrustManager[] trustManagers = tmf.getTrustManagers();
42+
context.init(null, trustManagers, null); // GOOD, we are not using a custom `TrustManager` but instead have
43+
// added the self-signed certificate we want to trust to the key
44+
// store. Note, the `trustManagers` will **only** trust this one
45+
// certificate.
46+
URL url = new URL("https://self-signed.badssl.com/");
47+
HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
48+
conn.setSSLSocketFactory(context.getSocketFactory());
49+
}
50+
}

‎java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.qhelp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
If the <code>checkServerTrusted</code> method of a <code>TrustManager</code> never throws a <code>CertificateException</code> it trusts every certificate.
8+
This allows an attacker to perform a Man-in-the-middle attack against the application therefore breaking any security Transport Layer Security (TLS) gives.
9+
10+
An attack would look like this:
11+
1. The program connects to <code>https://example.com</code>.
12+
2. The attacker intercepts this connection and presents a valid, self-signed certificate for <code>https://example.com</code>.
13+
3. Java calls the <code>checkServerTrusted</code> method to check whether it should trust the certificate.
14+
4. The <code>checkServerTrusted</code> method of your <code>TrustManager</code> does not throw a <code>CertificateException</code>.
15+
5. Java proceeds with the connection since your <code>TrustManager</code> implicitly trusted it by not throwing an exception.
16+
6. The attacker can now read the data your program sends to <code>https://example.com</code> and/or alter its replies while the program thinks the connection is secure.
17+
</p>
18+
</overview>
19+
20+
<recommendation>
21+
<p>
22+
Do not use a custom <code>TrustManager</code> that trusts any certificate.
23+
If you have to use a self-signed certificate, don't trust every certificate, but instead only trust this specific certificate.
24+
See below for an example of how to do this.
25+
</p>
26+
27+
</recommendation>
28+
29+
<example>
30+
<p>
31+
In the first (bad) example, the <code>TrustManager</code> never throws a <code>CertificateException</code> thereby trusting any certificate.
32+
This allows an attacker to perform a man-in-the-middle attack.
33+
In the second (good) example, no custom <code>TrustManager</code> is used. Instead, the self-signed certificate that should be trusted
34+
is explicitly trusted by loading it into a <code>KeyStore</code>.
35+
</p>
36+
<sample src="InsecureTrustManager.java" />
37+
</example>
38+
39+
<references>
40+
<li><a href="https://developer.android.com/training/articles/security-ssl">Android Security Guide for TLS/HTTPS</a>.</li>
41+
<li>OWASP: <a href="https://cwe.mitre.org/data/definitions/295.html">CWE-295</a>.</li>
42+
</references>
43+
</qhelp>

‎java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/**
2+
* @name Everything trusting `TrustManager`
3+
* @description Trusting all certificates allows an attacker to perform a machine-in-the-middle attack.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id java/insecure-trustmanager
8+
* @tags security
9+
* external/cwe/cwe-295
10+
*/
11+
12+
import java
13+
import semmle.code.java.controlflow.Guards
14+
import semmle.code.java.dataflow.DataFlow
15+
import semmle.code.java.dataflow.FlowSources
16+
import semmle.code.java.dataflow.TaintTracking2
17+
import semmle.code.java.security.Encryption
18+
import DataFlow::PathGraph
19+
20+
/**
21+
* Models an insecure `X509TrustManager`.
22+
* An `X509TrustManager` is considered insecure if it never throws a `CertificatException` thereby accepting any certificate as valid.
23+
*/
24+
class InsecureX509TrustManager extends RefType {
25+
InsecureX509TrustManager() {
26+
getASupertype*() instanceof X509TrustManager and
27+
exists(Method m |
28+
m.getDeclaringType() = this and
29+
m.hasName("checkServerTrusted") and
30+
not mayThrowCertificateException(m)
31+
)
32+
}
33+
}
34+
35+
/** The `java.security.cert.CertificateException` class. */
36+
private class CertificatException extends RefType {
37+
CertificatException() { hasQualifiedName("java.security.cert", "CertificateException") }
38+
}
39+
40+
/**
41+
*Holds if:
42+
* - `m` may `throw` an `CertificatException`
43+
* - `m` calls another method that may throw
44+
* - `m` calls a method that declares to throw an `CertificatExceptio`, but for which no source is available
45+
*/
46+
private predicate mayThrowCertificateException(Method m) {
47+
exists(Stmt stmt | m.getBody().getAChild*() = stmt |
48+
stmt.(ThrowStmt).getThrownExceptionType().getASupertype*() instanceof CertificatException
49+
)
50+
or
51+
exists(Method otherMethod | m.polyCalls(otherMethod) |
52+
mayThrowCertificateException(otherMethod)
53+
or
54+
not otherMethod.fromSource() and
55+
otherMethod.getAnException().getType().getASupertype*() instanceof CertificatException
56+
)
57+
}
58+
59+
/**
60+
* A configuration to model the flow of a `InsecureX509TrustManager` to an `SSLContext.init` call.
61+
*/
62+
class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
63+
InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
64+
65+
override predicate isSource(DataFlow::Node source) {
66+
source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof InsecureX509TrustManager
67+
}
68+
69+
override predicate isSink(DataFlow::Node sink) {
70+
exists(MethodAccess ma, Method m |
71+
m.hasName("init") and
72+
m.getDeclaringType() instanceof SSLContext and
73+
ma.getMethod() = m
74+
|
75+
ma.getArgument(1) = sink.asExpr()
76+
)
77+
}
78+
79+
override predicate isSanitizer(DataFlow::Node barrier) {
80+
// ignore nodes that are in functions that intentionally trust all certificates
81+
barrier
82+
.getEnclosingCallable()
83+
.getName()
84+
/*
85+
* Regex: (_)* :
86+
* some methods have underscores.
87+
* Regex: (no|ignore|disable)(strictssl|ssl|verify|verification)
88+
* noStrictSSL ignoreSsl
89+
* Regex: (set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)
90+
* acceptAll trustAll ignoreAll setTrustAnyHttps
91+
* Regex: (use|do|enable)insecure
92+
* useInsecureSSL
93+
* Regex: (set|do|use)?no.*(check|validation|verify|verification)
94+
* setNoCertificateCheck
95+
* Regex: disable
96+
* disableChecks
97+
*/
98+
99+
.regexpMatch("^(?i)(_)*((no|ignore|disable)(strictssl|ssl|verify|verification)" +
100+
"|(set)?(accept|trust|ignore|allow)(all|every|any|selfsigned)" +
101+
"|(use|do|enable)insecure|(set|do|use)?no.*(check|validation|verify|verification)|disable).*$")
102+
}
103+
}
104+
105+
bindingset[result]
106+
private string getAFlagName() {
107+
result
108+
.regexpMatch("(?i).*(secure|disable|selfCert|selfSign|validat|verif|trust|ignore|nocertificatecheck).*")
109+
}
110+
111+
/**
112+
* A flag has to either be of type `String`, `boolean` or `Boolean`.
113+
*/
114+
private class FlagType extends Type {
115+
FlagType() {
116+
this instanceof TypeString
117+
or
118+
this instanceof BooleanType
119+
}
120+
}
121+
122+
private predicate isEqualsIgnoreCaseMethodAccess(MethodAccess ma) {
123+
ma.getMethod().hasName("equalsIgnoreCase") and
124+
ma.getMethod().getDeclaringType() instanceof TypeString
125+
}
126+
127+
/** Holds if `source` should is considered a flag. */
128+
private predicate isFlag(DataFlow::Node source) {
129+
exists(VarAccess v | v.getVariable().getName() = getAFlagName() |
130+
source.asExpr() = v and v.getType() instanceof FlagType
131+
)
132+
or
133+
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | source.asExpr() = s)
134+
or
135+
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
136+
source.asExpr() = ma and
137+
ma.getType() instanceof FlagType and
138+
not isEqualsIgnoreCaseMethodAccess(ma)
139+
)
140+
}
141+
142+
/** Holds if there is flow from `node1` to `node2` either due to local flow or due to custom flow steps. */
143+
private predicate flagFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
144+
DataFlow::localFlowStep(node1, node2)
145+
or
146+
exists(MethodAccess ma | ma.getMethod() = any(EnvReadMethod m) |
147+
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
148+
)
149+
or
150+
exists(MethodAccess ma |
151+
ma.getMethod().hasName("parseBoolean") and
152+
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Boolean")
153+
|
154+
ma = node2.asExpr() and ma.getAnArgument() = node1.asExpr()
155+
)
156+
}
157+
158+
/** Gets a guard that depends on a flag. */
159+
private Guard getAGuard() {
160+
exists(DataFlow::Node source, DataFlow::Node sink |
161+
isFlag(source) and
162+
flagFlowStep*(source, sink) and
163+
sink.asExpr() = result
164+
)
165+
}
166+
167+
/** Holds if `node` is guarded by a flag that suggests an intentionally insecure feature. */
168+
private predicate isNodeGuardedByFlag(DataFlow::Node node) {
169+
exists(Guard g | g.controls(node.asExpr().getBasicBlock(), _) | g = getAGuard())
170+
}
171+
172+
from
173+
DataFlow::PathNode source, DataFlow::PathNode sink, InsecureTrustManagerConfiguration cfg,
174+
RefType trustManager
175+
where
176+
cfg.hasFlowPath(source, sink) and
177+
not isNodeGuardedByFlag(sink.getNode()) and
178+
trustManager = source.getNode().asExpr().(ClassInstanceExpr).getConstructedType()
179+
select sink, source, sink, "$@ that is defined $@ and trusts any certificate, is used here.",
180+
source, "This trustmanager", trustManager, "here"

‎java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
edges
2+
| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager |
3+
| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:49:24:49:35 | trustManager |
4+
nodes
5+
| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
6+
| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | semmle.label | trustManager |
7+
| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
8+
| InsecureTrustManagerTest.java:49:24:49:35 | trustManager | semmle.label | trustManager |
9+
#select
10+
| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:20:23:20:42 | InsecureTrustManager | here |

‎java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.qlref

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

‎java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import java.io.File;
2+
import java.io.FileInputStream;
3+
import java.io.InputStream;
4+
import java.net.URL;
5+
import java.security.KeyStore;
6+
import java.security.cert.CertificateException;
7+
import java.security.cert.CertificateFactory;
8+
import java.security.cert.X509Certificate;
9+
10+
import javax.net.ssl.HttpsURLConnection;
11+
import javax.net.ssl.SSLContext;
12+
import javax.net.ssl.TrustManager;
13+
import javax.net.ssl.TrustManagerFactory;
14+
import javax.net.ssl.X509TrustManager;
15+
16+
public class InsecureTrustManagerTest {
17+
18+
private static final boolean TRUST_ALL = true;
19+
20+
private static class InsecureTrustManager implements X509TrustManager {
21+
@Override
22+
public X509Certificate[] getAcceptedIssuers() {
23+
return null;
24+
}
25+
26+
@Override
27+
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
28+
// BAD: Does not verify the certificate chain, allowing any certificate.
29+
}
30+
31+
@Override
32+
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
33+
34+
}
35+
}
36+
37+
public static void main(String[] args) throws Exception {
38+
{
39+
SSLContext context = SSLContext.getInstance("TLS");
40+
TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
41+
context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the certificate
42+
// chain, allowing any certificate.
43+
}
44+
45+
{
46+
if (TRUST_ALL) {
47+
SSLContext context = SSLContext.getInstance("TLS");
48+
TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
49+
context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the
50+
// certificate
51+
// chain, allowing any certificate. BUT it is guarded
52+
// by a feature flag.
53+
}
54+
}
55+
{
56+
disableTrustManager();
57+
}
58+
{
59+
SSLContext context = SSLContext.getInstance("TLS");
60+
File certificateFile = new File("path/to/self-signed-certificate");
61+
// Create a `KeyStore` with default type
62+
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
63+
// This causes `keyStore` to be empty
64+
keyStore.load(null, null);
65+
X509Certificate generatedCertificate;
66+
try (InputStream cert = new FileInputStream(certificateFile)) {
67+
generatedCertificate = (X509Certificate) CertificateFactory.getInstance("X509")
68+
.generateCertificate(cert);
69+
}
70+
// Add the self-signed certificate to the key store
71+
keyStore.setCertificateEntry(certificateFile.getName(), generatedCertificate);
72+
// Get default `TrustManagerFactory`
73+
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
74+
// Use it with our modified key store that trusts our self-signed certificate
75+
tmf.init(keyStore);
76+
TrustManager[] trustManagers = tmf.getTrustManagers();
77+
context.init(null, trustManagers, null); // GOOD, we are not using a custom `TrustManager` but instead have
78+
// added the self-signed certificate we want to trust to the key
79+
// store. Note, the `trustManagers` will **only** trust this one
80+
// certificate.
81+
URL url = new URL("https://self-signed.badssl.com/");
82+
HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
83+
conn.setSSLSocketFactory(context.getSocketFactory());
84+
}
85+
}
86+
87+
private static void disableTrustManager() {
88+
SSLContext context = SSLContext.getInstance("TLS");
89+
TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
90+
context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the
91+
// certificate
92+
// chain, allowing any certificate. BUT it is the method name suggests that this
93+
// is intentional.
94+
}
95+
}

0 commit comments

Comments
 (0)