Skip to content

Java: Add unsafe hostname verification query and remove existing overlapping query #4771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aschackmull merged 25 commits into from
Jan 13, 2021
Merged

Java: Add unsafe hostname verification query and remove existing overlapping query #4771

Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8df5d77
Java: Model `HostnameVerifier` method
intrigus Dec 4, 2020
3da1cb0
Java: Add unsafe hostname verification query
intrigus Dec 4, 2020
70b0703
Java: Remove overlapping code
intrigus Dec 4, 2020
0a9df07
Apply suggestions from review.
intrigus Dec 9, 2020
e021158
Java: Tighter model of `HostnameVerifier#verify`
intrigus Dec 9, 2020
d98b171
Java: Make `EnvTaintedMethod` public + QL-Doc
intrigus Dec 9, 2020
a62a2e5
Java: Improve QL-Doc
intrigus Dec 9, 2020
9e2ef9b
Java: Filter results by feature flags.
intrigus Dec 9, 2020
33b0ff2
Java: Update test
intrigus Dec 9, 2020
c88f07d
Java: Accept test output
intrigus Dec 9, 2020
10fc2cf
Apply suggestions from code review
intrigus-lgtm Dec 14, 2020
355cb6e
Fix Qhelp format
intrigus-lgtm Dec 16, 2020
502e4c3
Java: Fix Qhelp
intrigus Dec 17, 2020
b8f3e64
Apply suggestions from code review
intrigus-lgtm Jan 4, 2021
e11304a
Java: Autoformat
intrigus Jan 5, 2021
f4b912c
Apply suggestions from doc review
intrigus-lgtm Jan 9, 2021
b469273
Java: Add QLDoc improve query message
intrigus Jan 7, 2021
1eb2b75
Java: Further reduce FPs, simply Flag2Guard flow
intrigus Jan 10, 2021
5c1e746
Java: Rename to `EnvReadMethod`
intrigus Jan 11, 2021
4cfdb10
Java: Improve QLDoc & simplify code
intrigus-lgtm Jan 11, 2021
722bd4d
Java: Revise qhelp
intrigus-lgtm Jan 11, 2021
85286f3
Java: Replace global flow by local flow
intrigus Jan 11, 2021
4fa8f5e
Java: Accept test changes
intrigus Jan 12, 2021
1901f6b
Java: Make @id @name of query more similar.
intrigus Jan 12, 2021
2931e1f
Java: Add change note for #4771
intrigus Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply suggestions from doc review 
Co-authored-by: Felicity Chapman <felicitymay@github.com>
  • Loading branch information
@intrigus-lgtm @felicitymay @intrigus
2 people authored and intrigus committed Jan 11, 2021
commit f4b912cd8a949a3a5e1c1428603293f2a9a152e0
10 changes: 5 additions & 5 deletions java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<overview>
<p>
If a <code>HostnameVerifier</code> always returns <code>true</code> it will not verify the hostname at all.
This stops Transport Layer Security (TLS) providing any security and allows an attacker to perform a Man-in-the-middle attack against the application.
This stops Transport Layer Security (TLS) providing any security and allows an attacker to perform a man-in-the-middle attack against the application.
</p>

<p>
Expand All @@ -14,10 +14,10 @@ An attack might look like this:

<ol>
<li>The program connects to <code>https://example.com</code>.</li>
<li>The attacker intercepts this connection and presents one of their valid certificates they control, for example one from Let's Encrypt.</li>
<li>Java verifies that the certificate has been issued by a trusted certificate authority.</li>
<li>Java verifies that the certificate has been issued for the host <code>example.com</code>, which will fail because the certificate has been issued for <code>malicious.domain</code>.</li>
<li>Java wants to reject the certificate because the hostname does not match. Before doing this it checks whether there exists a <code>HostnameVerifier</code>.</li>
<li>The attacker intercepts this connection and presents an apparently-valid certificate of their choosing.</li>
<li>The `TrustManager` of the program verifies that the certificate has been issued by a trusted certificate authority.</li>
<li>Java checks whether the certificate has been issued for the host <code>example.com</code>. This check fails because the certificate has been issued for a domain controlled by the attacker, for example: <code>malicious.domain</code>.</li>
<li>Java wants to reject the certificate because the hostname does not match. Before doing this it checks whether a <code>HostnameVerifier</code> exists.</li>
Comment thread
intrigus-lgtm marked this conversation as resolved.
Outdated
<li>Your <code>HostnameVerifier</code> is called which returns <code>true</code> for any certificate so also for this one.</li>
Comment thread
felicitymay marked this conversation as resolved.
<li>Java proceeds with the connection since your <code>HostnameVerifier</code> accepted it.</li>
<li>The attacker can now read the data your program sends to <code>https://example.com</code>
Expand Down