Skip to content

Commit 4341325

Browse files
intrigusintrigus
committed
[Java] CWE-295 - Incorrect Hostname Verification
1 parent c54f8d8 commit 4341325

9 files changed

Lines changed: 292 additions & 0 deletions

‎java/ql/src/experimental/Security/CWE/CWE-295/AlwaysTrueHostnameVerifier.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
HostnameVerifier hostnameVerifier = new HostnameVerifier() {
2+
@Override
3+
public boolean verify(String hostname, SSLSession session) {
4+
return true; // BAD, verifies anything as correct
5+
}
6+
};

‎java/ql/src/experimental/Security/CWE/CWE-295/AndroidHostnameVerifier.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Create an HostnameVerifier that hardwires the expected hostname.
2+
// Note that is different than the URL's hostname:
3+
// example.com versus example.org
4+
HostnameVerifier hostnameVerifier = new HostnameVerifier() {
5+
@Override
6+
public boolean verify(String hostname, SSLSession session) {
7+
HostnameVerifier hv =
8+
HttpsURLConnection.getDefaultHostnameVerifier();
9+
return hv.verify("example.com", session); // OKAY-ISH: verify that the certificate matches
10+
// example.com and only accept example.com as an alternative to example.org
11+
// BETTER: fix the underlying configuration problem that causes example.org to present the certificate for the wrong domain.
12+
}
13+
};
14+
15+
// Tell the URLConnection to use our HostnameVerifier
16+
URL url = new URL("https://example.org/");
17+
HttpsURLConnection urlConnection =
18+
(HttpsURLConnection)url.openConnection();
19+
urlConnection.setHostnameVerifier(hostnameVerifier);
20+
InputStream in = urlConnection.getInputStream();

‎java/ql/src/experimental/Security/CWE/CWE-295/EverythingAcceptingHostnameVerifier.qhelp‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
If a HostnameVerifier always returns <code>true</code> it will not verify the hostname at all.
8+
This allows an attacker to perform a man-in-the-middle attack on the application therefore breaking any security Transport Layer Security (TLS) gives.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
DO NOT USE AN UNVERIFYING HostnameVerifier!
15+
<li>If you use an unverifying verifier to a solve configuration problem with TLS/HTTPS you should solve the configuration problem instead.
16+
</li>
17+
<li>If you use an unverifying verifier to connect to a local host you should instead create a self-signed certificate for your local host and import that certificate to the java key store</li>
18+
If you do not verify the hostname, you might as well not use TLS/HTTPS at all.
19+
</p>
20+
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
In the first example, the <code>HostnameVerifier</code> always returns <code>true</code>.
26+
This allows an attacker to perform a man-in-the-middle attack, because any certificate is accepted despite an incorrect hostname.
27+
</p>
28+
<sample src="AlwaysTrueHostnameVerifier.java" />
29+
</example>
30+
31+
<references>
32+
<li><a href="https://developer.android.com/training/articles/security-ssl">Android Security Guide for TLS/HTTPS</a>.</li>
33+
<li><a href="https://tersesystems.com/blog/2014/03/23/fixing-hostname-verification/">Further Information on Hostname Verification</a>.</li>
34+
<li>OWASP: <a href="https://cwe.mitre.org/data/definitions/295.html">CWE-295</a>.</li>
35+
</references>
36+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-295/EverythingAcceptingHostnameVerifier.ql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import HostnameValidation
2+
3+
from AlwaysAcceptingVerifyMethod m
4+
where not m.getDeclaringType() instanceof TestClass
5+
select m

‎java/ql/src/experimental/Security/CWE/CWE-295/HostnameValidation.qll‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
import java
2+
import semmle.code.java.dataflow.TaintTracking
3+
4+
/** The class `javax.net.ssl.HostnameVerifier`. */
5+
class TypeHostnameVerifier extends RefType {
6+
TypeHostnameVerifier() { hasQualifiedName("javax.net.ssl", "HostnameVerifier") }
7+
}
8+
9+
/** The `javax.net.ssl.HostnameVerifier.verify` method. */
10+
class HostnameVerifierVerifyMethod extends Method {
11+
HostnameVerifierVerifyMethod() {
12+
hasName("verify") and getDeclaringType() instanceof TypeHostnameVerifier
13+
}
14+
}
15+
16+
/** A method that overrides the `javax.net.ssl.HostnameVerifier.verify` method. */
17+
class OverridenVerifyMethod extends Method {
18+
OverridenVerifyMethod() {
19+
exists(HostnameVerifierVerifyMethod m | this.overridesOrInstantiates*(m))
20+
}
21+
22+
/**
23+
* The `hostname` parameter of this method.
24+
*/
25+
Parameter getHostnameParameter() { result = getParameter(0) }
26+
27+
/**
28+
* The `session` parameter of this method.
29+
*/
30+
Parameter getSessionParameter() { result = getParameter(1) }
31+
}
32+
33+
/** A return statement that returns `true`. */
34+
class TrueReturnStmt extends ReturnStmt {
35+
TrueReturnStmt() { getResult().(BooleanLiteral).getBooleanValue() = true }
36+
}
37+
38+
/**
39+
* Holds if `m` always returns `true` ignoring any exceptional flow.
40+
*/
41+
private predicate alwaysReturnsTrue(OverridenVerifyMethod m) {
42+
forex(ReturnStmt rs | rs.getEnclosingCallable() = m | rs instanceof TrueReturnStmt)
43+
}
44+
45+
/** A method that overrides the `javax.net.ssl.HostnameVerifier.verify` method in a dangerous way. */
46+
abstract class DangerousVerifyMethod extends OverridenVerifyMethod {
47+
/** A `Stmt` that makes this `HostnameVerifier` dangerous. */
48+
abstract Stmt getADangerousStmt();
49+
}
50+
51+
/**
52+
* A method that overrides the `javax.net.ssl.HostnameVerifier.verify` method and **always** returns `true`, thus
53+
* accepting any certificate despite a hostname mismatch.
54+
*/
55+
class AlwaysAcceptingVerifyMethod extends DangerousVerifyMethod {
56+
AlwaysAcceptingVerifyMethod() { alwaysReturnsTrue(this) }
57+
58+
override Stmt getADangerousStmt() {
59+
exists(TrueReturnStmt r | r.getEnclosingCallable() = this | result = r)
60+
}
61+
}
62+
63+
/** An access to any method whose name starts with `equals` and which is defined in the `String` class. */
64+
class StringEqualsMethodAccess extends MethodAccess {
65+
StringEqualsMethodAccess() {
66+
getMethod().getName().matches("equals%") and
67+
getMethod().getDeclaringType() instanceof TypeString
68+
}
69+
70+
// TODO there is probably a better name for "anEqualityPart" but I don't know any right now
71+
/** Returns a part of this equality check, i.e. in the case of `foo.equals(bar)` this returns `foo` and `bar`. */
72+
Expr getAnEqualityPart() { result = getQualifier() or result = getArgument(0) }
73+
}
74+
75+
/**
76+
* Get the nearest enclosing if statement, if there exists one.
77+
* For example consider this case:
78+
* ````
79+
* if(foo) {
80+
* if(bar) {
81+
* stmt
82+
* }
83+
* }
84+
* ````
85+
* This query would then return the `bar` if statement and **not** the `foo` if statement.
86+
*/
87+
private IfStmt getNearestEnclosingIfStmt(Stmt stmt) {
88+
exists(IfStmt ifStmt |
89+
result = ifStmt and
90+
ifStmt.getAChild+() = stmt and
91+
/*
92+
* Ensure that the `IfStmt` we found does not contain another `IfStmt`.
93+
* If it contains one, the result can not be the nearest enclosing `IfStmt`.
94+
*/
95+
96+
not exists(IfStmt enclosedIfStmt | enclosedIfStmt = ifStmt.getAChild+())
97+
)
98+
}
99+
100+
/** The `javax.net.ssl.SSLSession.getPeerHost` method. */
101+
private class SslSessionGetPeerHostMethodAccess extends MethodAccess {
102+
SslSessionGetPeerHostMethodAccess() {
103+
getMethod().hasName("getPeerHost") and
104+
getMethod().getDeclaringType().hasQualifiedName("javax.net.ssl", "SSLSession")
105+
}
106+
}
107+
108+
/**
109+
* A (dangerous) `StringEqualsMethodAccess` where the equality does:
110+
* - not depend on a expression that is dervied from the `session` parameter of the `verify` method.
111+
* - not depend on the value of `SSLSession.getPeerHost`, since the value is not authenticated.
112+
*/
113+
class SslSessionParameterIgnoringStringEqualsMethodAccess extends StringEqualsMethodAccess {
114+
SslSessionParameterIgnoringStringEqualsMethodAccess() {
115+
exists(Expr e, Parameter hostnameParameter, Parameter sslSessionParameter |
116+
hostnameParameter = getEnclosingCallable().(OverridenVerifyMethod).getHostnameParameter() and
117+
sslSessionParameter = getEnclosingCallable().(OverridenVerifyMethod).getSessionParameter() and
118+
getAnEqualityPart().getAChildExpr*() = e
119+
|
120+
not TaintTracking::localExprTaint(sslSessionParameter.getAnAccess(), e) and
121+
not e instanceof SslSessionGetPeerHostMethodAccess
122+
)
123+
}
124+
}
125+
126+
/**
127+
* A `verify` method that does not verify the `hostname` correctly. It is incorrect
128+
* because it does not validate the `hostname` against a
129+
* any expression that is dervied from the `session` parameter of the `verify` method.
130+
*/
131+
class IncorrectHostnameVerifyMethod extends DangerousVerifyMethod {
132+
Stmt dangerousStmt;
133+
134+
IncorrectHostnameVerifyMethod() {
135+
/* We return `true` based on an if statement that verifies the `hostname` incorrectly.*/
136+
exists(TrueReturnStmt r, IfStmt i |
137+
getBody().getAChild*() = r and i = getNearestEnclosingIfStmt(r)
138+
|
139+
i.getCondition().getAChildExpr*() instanceof
140+
SslSessionParameterIgnoringStringEqualsMethodAccess and
141+
dangerousStmt = i
142+
)
143+
or
144+
/* We return the result of an `equals` call that verifies the `hostname` incorrectly.*/
145+
exists(ReturnStmt r | getBody().getAChild*() = r |
146+
not r.getResult() instanceof BooleanLiteral and
147+
r.getResult().getAChildExpr*() instanceof SslSessionParameterIgnoringStringEqualsMethodAccess and
148+
dangerousStmt = r
149+
)
150+
}
151+
152+
override Stmt getADangerousStmt() { result = dangerousStmt }
153+
}

‎java/ql/src/experimental/Security/CWE/CWE-295/IncorrectHostnameVerifier.qhelp‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
If a HostnameVerifier always returns <code>true</code> it will not verify the hostname at all.
8+
This allows an attacker to perform a man-in-the-middle attack on the application therefore breaking any security Transport Layer Security (TLS) gives.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
DO NOT USE AN UNVERIFYING HostnameVerifier!
15+
<li>If you use an unverifying verifier to a solve configuration problem with TLS/HTTPS you should solve the configuration problem instead.
16+
</li>
17+
<li>If you use an unverifying verifier to connect to a local host you should instead create a self-signed certificate for your local host and import that certificate to the java key store</li>
18+
If you do not verify the hostname, might as well not use TLS/HTTPS at all.
19+
<li>If the above solutions do not work you can implement a custom HostnameVerifier, if you **understand** the security impact of your implementation.</li>
20+
<li>Ensure that you do not verify anything against the return value of the <code>session.getPeerHost()</code> method since it is not authenticated.</li>
21+
Ensure that you at least verify the given <code>hostname</code> against the values that can be derived from the return value of the <code>session.getPeerCertificates()</code> method.
22+
Verifying <code>hostname</code> against anything that is **not** derived from <code>session.getPeerCertificates()</code> usually is insecure.
23+
Ensure that you use the default hostname verifier when using Android instead of verifying it yourself.
24+
The default (OpenJDK) Java hostname verifier can not be used, since it always returns <code>false</code>.
25+
</li>
26+
</p>
27+
28+
</recommendation>
29+
30+
<example>
31+
<p>
32+
In the first example, the <code>HostnameVerifier</code> always returns <code>true</code>.
33+
This allows an attacker to perform a man-in-the-middle attack, because any certificate is accepted despite an incorrect hostname.
34+
</p>
35+
<sample src="AlwaysTrueHostnameVerifier.java" />
36+
37+
<p>
38+
In the second example, we want to connect to the example.org host, but the host presents a certificate for example.com.
39+
Assuming that example.org and example.com are under our control (and only then this is ok) we can use the default <code>HostnameVerifier</code>
40+
to verify that the certificate belongs to example.com.
41+
Note that you should really fix the configuration problem that causes example.org to present the certificate for example.com instead of using this method.
42+
This method **only** works on Android, on (OpenJDK) Java the default <code>HostnameVerifier</code> will always return <code>false</code>.
43+
On (OpenJDK) Java you can use <a href="https://github.com/AsyncHttpClient/async-http-client/commit/3c9152e">this implementation</a> of a working
44+
hostname verifier instead of the default <code>HostnameVerifier</code>.
45+
</p>
46+
<sample src="AndroidHostnameVerifier.java" />
47+
</example>
48+
49+
<references>
50+
<li><a href="https://developer.android.com/training/articles/security-ssl">Android Security Guide for TLS/HTTPS</a>.</li>
51+
<li><a href="https://tersesystems.com/blog/2014/03/23/fixing-hostname-verification/">Further Information on Hostname Verification</a>.</li>
52+
<li>OWASP: <a href="https://cwe.mitre.org/data/definitions/295.html">CWE-295</a>.</li>
53+
</references>
54+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-295/IncorrectHostnameVerifier.ql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import HostnameValidation
2+
3+
from IncorrectHostnameVerifyMethod m
4+
where not m.getDeclaringType() instanceof TestClass
5+
select m.getADangerousStmt(), "This statement incorecctly verifies the hostname."

‎java/ql/src/experimental/Security/CWE/CWE-295/LocalhostHostnameVerifier.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
HostnameVerifier hostnameVerifier = new HostnameVerifier() {
2+
@Override
3+
public boolean verify(String hostname, SSLSession session) {
4+
return hostname.equals("localhost"); // BAD, does not verify the certificate
5+
}
6+
};

‎java/ql/src/experimental/Security/CWE/CWE-295/PeerHostHostnameVerifier.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
HostnameVerifier hostnameVerifier = new HostnameVerifier() {
2+
@Override
3+
public boolean verify(String hostname, SSLSession session) {
4+
return hostname.equals(session.getPeerHost()); // BAD, getPeerHost() is not authenticated
5+
// and will always be equal to the hostname, therefore trusting ALL hostnames
6+
}
7+
};

0 commit comments

Comments
 (0)