Merge pull request #6443 from artem-smotrakov/ignored-hostname-verifier

Java: An experimental query for ignored hostname verification

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;
+}

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;
+}

java/ql/src/experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+The method <code>HostnameVerifier.verify()</code> checks that the hostname from the server's certificate
+matches the server hostname after an HTTPS connection is established.
+The method returns <code>true</code> if the hostname is acceptable and <code>false</code> 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.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Always check the result of <code>HostnameVerifier.verify()</code> and drop the connection
+if the method returns false.
+</p>
+</recommendation>
+
+<example>
+<p>
+In the following example, the method <code>HostnameVerifier.verify()</code> is called but its result is ignored.
+As a result, no hostname verification actually happens.
+</p>
+<sample src="IgnoredHostnameVerification.java" />
+
+<p>
+In the next example, the result of the <code>HostnameVerifier.verify()</code> method is checked
+and an exception is thrown if the verification failed.
+</p>
+<sample src="CheckedHostnameVerification.java" />
+</example>
+
+<references>
+<li>
+  Java API Specification:
+  <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/javax/net/ssl/HostnameVerifier.html#verify(java.lang.String,javax.net.ssl.SSLSession)">HostnameVerifier.verify() method</a>.
+</li>
+</references>
+</qhelp>

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."

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. |

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
+    }
+
+  }
+
+}

java/ql/test/experimental/query-tests/security/CWE-297/IgnoredHostnameVerification.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-297/IgnoredHostnameVerification.ql