From 10fc2cf9f803acf2c4d5eb34071ad2b6a1e16e87 Mon Sep 17 00:00:00 2001
From: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com>
Date: Mon, 14 Dec 2020 15:51:53 +0100
Subject: [PATCH] Apply suggestions from code review
Co-authored-by: Chris Smowton
---
.../Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp | 8 ++++----
.../Security/CWE/CWE-297/UnsafeHostnameVerification.ql | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp
index 71c28a93159..f8bfc87a9c3 100644
--- a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp
+++ b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.qhelp
@@ -15,14 +15,14 @@ An attack would look like this:
5. Java wants to reject the certificate because the hostname does not match. Before doing this it checks whether there exists a HostnameVerifier.
6. Your HostnameVerifier is called which returns true for any certificate so also for this one.
7. Java proceeds with the connection since your HostnameVerifier accepted it.
-8. The attacker can now read the data (Man-in-the-middle) your program sends to https://example.com while the program thinks the connection is secure.
+8. The attacker can now read the data your program sends to https://example.com and/or alter its replies while the program thinks the connection is secure.
-Do NOT use an unverifying HostnameVerifier!
-
If you use an unverifying verifier to solve a configuration problem with TLS/HTTPS you should solve the configuration problem instead.
+Do not use an open HostnameVerifier.
+If you use an open verifier to solve a configuration problem with TLS/HTTPS you should solve the configuration problem instead.
@@ -42,4 +42,4 @@ In the second (good) example, the HostnameVerifier only returns Further Information on Hostname Verification.
OWASP: CWE-297.
-
\ No newline at end of file
+
diff --git a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql
index 2e47f501f7b..d8960558894 100644
--- a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql
+++ b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql
@@ -1,6 +1,6 @@
/**
* @name Disabled hostname verification
- * @description Accepting any certificate as valid for a host allows an attacker to perform a man-in-the-middle attack.
+ * @description Accepting any certificate as valid for a host allows an attacker to perform a machine-in-the-middle attack.
* @kind path-problem
* @problem.severity error
* @precision high
@@ -29,7 +29,7 @@ private predicate alwaysReturnsTrue(HostnameVerifierVerify m) {
}
/**
- * A class that overrides the `javax.net.ssl.HostnameVerifier.verify` method and **always** returns `true` (ignoring exceptional flow), thus
+ * A class that overrides the `javax.net.ssl.HostnameVerifier.verify` method and **always** returns `true` (though it could also exit due to an uncaught exception), thus
* accepting any certificate despite a hostname mismatch.
*/
class TrustAllHostnameVerifier extends RefType {