diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java b/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java new file mode 100644 index 000000000000..9f17b1fc9726 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-297/CheckedHostnameVerification.java @@ -0,0 +1,10 @@ +public SSLSocket connect(String host, int port, HostnameVerifier verifier) { + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + boolean successful = verifier.verify(host, socket.getSession()); + if (!successful) { + socket.close(); + throw new SSLException("Oops! Hostname verification failed!"); + } + return socket; +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java new file mode 100644 index 000000000000..25436051dbca --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.java @@ -0,0 +1,6 @@ +public SSLSocket connect(String host, int port, HostnameVerifier verifier) { + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + verifier.verify(host, socket.getSession()); + return socket; +} \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp new file mode 100644 index 000000000000..e5756d9caee9 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp @@ -0,0 +1,42 @@ + + + + +

+The method HostnameVerifier.verify() checks that the hostname from the server's certificate +matches the server hostname after an HTTPS connection is established. +The method returns true if the hostname is acceptable and false otherwise. The contract of the method +does not require it to throw an exception if the verification failed. +Therefore, a caller has to check the result and drop the connection if the hostname verification failed. +Otherwise, an attacker may be able to implement a man-in-the-middle attack and impersonate the server. +

+
+ + +

+Always check the result of HostnameVerifier.verify() and drop the connection +if the method returns false. +

+
+ + +

+In the following example, the method HostnameVerifier.verify() is called but its result is ignored. +As a result, no hostname verification actually happens. +

+ + +

+In the next example, the result of the HostnameVerifier.verify() method is checked +and an exception is thrown if the verification failed. +

+ +
+ + +
  • + Java API Specification: + HostnameVerifier.verify() method. +
  • +
    +
    diff --git a/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql new file mode 100644 index 000000000000..55d51a19a8c9 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql @@ -0,0 +1,29 @@ +/** + * @name Ignored result of hostname verification + * @description The method HostnameVerifier.verify() returns a result of hostname verification. + * A caller has to check the result and drop the connection if the verification failed. + * @kind problem + * @problem.severity error + * @precision high + * @id java/ignored-hostname-verification + * @tags security + * external/cwe/cwe-297 + */ + +import java +import semmle.code.java.security.Encryption + +/** A `HostnameVerifier.verify()` call that is not wrapped in another `HostnameVerifier`. */ +private class HostnameVerificationCall extends MethodAccess { + HostnameVerificationCall() { + this.getMethod() instanceof HostnameVerifierVerify and + not this.getCaller() instanceof HostnameVerifierVerify + } + + /** Holds if the result of the call is not used. */ + predicate isIgnored() { this = any(ExprStmt es).getExpr() } +} + +from HostnameVerificationCall verification +where verification.isIgnored() +select verification, "Ignored result of hostname verification." diff --git a/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.expected b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.expected new file mode 100644 index 000000000000..579da26bf217 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.expected @@ -0,0 +1 @@ +| IgnoredHostnameVerification.java:16:5:16:46 | verify(...) | Ignored result of hostname verification. | \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.java b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.java new file mode 100644 index 000000000000..f79fd15af232 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.java @@ -0,0 +1,112 @@ +import java.io.IOException; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +public class IgnoredHostnameVerification { + + // BAD: ignored result of HostnameVerifier.verify() + public static SSLSocket connectWithIgnoredHostnameVerification( + String host, int port, HostnameVerifier verifier) throws IOException { + + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + verifier.verify(host, socket.getSession()); + return socket; + } + + public static void check(boolean result) throws SSLException { + if (!result) { + throw new SSLException("Oops! Hostname verification failed!"); + } + } + + // GOOD: connect and check result of HostnameVerifier.verify() + public static SSLSocket connectWithHostnameVerification00( + String host, int port, HostnameVerifier verifier) throws IOException { + + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + check(verifier.verify(host, socket.getSession())); + return socket; + } + + // GOOD: connect and check result of HostnameVerifier.verify() + public static SSLSocket connectWithHostnameVerification01( + String host, int port, HostnameVerifier verifier) throws IOException { + + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + boolean successful = verifier.verify(host, socket.getSession()); + if (successful == false) { + socket.close(); + throw new SSLException("Oops! Hostname verification failed!"); + } + + return socket; + } + + // GOOD: connect and check result of HostnameVerifier.verify() + public static SSLSocket connectWithHostnameVerification02( + String host, int port, HostnameVerifier verifier) throws IOException { + + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + boolean successful = false; + if (verifier != null) { + successful = verifier.verify(host, socket.getSession()); + } + if (!successful) { + socket.close(); + throw new SSLException("Oops! Hostname verification failed!"); + } + + return socket; + } + + // GOOD: connect and check result of HostnameVerifier.verify() + public static SSLSocket connectWithHostnameVerification03( + String host, int port, HostnameVerifier verifier) throws IOException { + + SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port); + socket.startHandshake(); + boolean successful = verifier.verify(host, socket.getSession()); + if (successful) { + return socket; + } + + socket.close(); + throw new SSLException("Oops! Hostname verification failed!"); + } + + // GOOD: connect and check result of HostnameVerifier.verify() + public static String connectWithHostnameVerification04( + String[] hosts, HostnameVerifier verifier, SSLSession session) throws IOException { + + for (String host : hosts) { + if (verifier.verify(host, session)) { + return host; + } + } + + throw new SSLException("Oops! Hostname verification failed!"); + } + + public static class HostnameVerifierWrapper implements HostnameVerifier { + + private final HostnameVerifier verifier; + + public HostnameVerifierWrapper(HostnameVerifier verifier) { + this.verifier = verifier; + } + + @Override + public boolean verify(String hostname, SSLSession session) { + return verifier.verify(hostname, session); // GOOD: wrapped calls should not be reported + } + + } + +} \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.qlref b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.qlref new file mode 100644 index 000000000000..454b421f7b24 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql \ No newline at end of file