Updated IgnoredHostnameVerification.ql to cover more uses of HostnameVerifier.verify()

Author: artem-smotrakov

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

@@ -4,15 +4,13 @@
  *              A caller has to check the result and drop the connection if the verification failed.
  * @kind problem
  * @problem.severity error
- * @precision medium
+ * @precision high
  * @id java/ignored-hostname-verification
  * @tags security
  *       external/cwe/cwe-297
  */
 
 import java
-import semmle.code.java.controlflow.Guards
-import semmle.code.java.dataflow.DataFlow
 
 /** The `HostnameVerifier.verify()` method. */
 private class HostnameVerifierVerifyMethod extends Method {
@@ -22,45 +20,17 @@ private class HostnameVerifierVerifyMethod extends Method {
   }
 }
 
-/** Defines `HostnameVerifier.verity()` calls that are not wrapped by another `HostnameVerifier`. */
+/** Defines `HostnameVerifier.verity()` calls that is not wrapped in another `HostnameVerifier`. */
 private class HostnameVerificationCall extends MethodAccess {
   HostnameVerificationCall() {
     this.getMethod() instanceof HostnameVerifierVerifyMethod and
     not this.getCaller() instanceof HostnameVerifierVerifyMethod
   }
 
-  /** Holds if the result if the call is not useds. */
+  /** Holds if the result of the call is not used. */
   predicate isIgnored() {
-    not exists(
-      DataFlow::Node source, DataFlow::Node sink, CheckFailedHostnameVerificationConfig config
-    |
-      this = source.asExpr() and config.hasFlow(source, sink)
-    )
-  }
-}
-
-/**
- * A configuration that tracks data flows from the result of a `HostnameVerifier.vefiry()` call
- * to a condition that controls a throw statement.
- */
-private class CheckFailedHostnameVerificationConfig extends DataFlow::Configuration {
-  CheckFailedHostnameVerificationConfig() { this = "CheckFailedHostnameVerificationConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof HostnameVerificationCall
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
-    exists(Guard guard, ThrowStmt throwStmt, ReturnStmt returnStmt |
-      (
-        guard.controls(throwStmt.getBasicBlock(), false) or
-        guard.controls(returnStmt.getBasicBlock(), true)
-      ) and
-      (
-        guard = sink.asExpr() or
-        guard.(EqualityTest).getAnOperand() = sink.asExpr() or
-        guard.(HostnameVerificationCall) = sink.asExpr()
-      )
+    not exists(Expr expr, IfStmt ifStmt, MethodAccess ma |
+      this = [expr.getAChildExpr(), ifStmt.getCondition(), ma.getAnArgument()]
     )
   }
 }

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

@@ -1,3 +1 @@
-| IgnoredHostnameVerification.java:16:5:16:46 | verify(...) | Ignored result of hostname verification. |
-| IgnoredHostnameVerification.java:26:22:26:63 | verify(...) | Ignored result of hostname verification. |
-| IgnoredHostnameVerification.java:37:22:37:63 | verify(...) | Ignored result of hostname verification. |
+| IgnoredHostnameVerification.java:16:5:16:46 | verify(...) | Ignored result of hostname verification. |

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

@@ -17,28 +17,19 @@ public static SSLSocket connectWithIgnoredHostnameVerification(
     return socket;
   }
 
-  // BAD: ignored result of HostnameVerifier.verify()
-  public static SSLSocket connectAndOnlyPrintResultOfHostnameVerification(
-      String host, int port, HostnameVerifier verifier) throws IOException {
-
-    SSLSocket socket = (SSLSocket) SSLSocketFactory.getDefault().createSocket(host, port);
-    socket.startHandshake();
-    boolean result = verifier.verify(host, socket.getSession());
-    System.out.println("Result of hostname verification: " + result);
-    return socket;
+  public static void check(boolean result) throws SSLException {
+    if (!result) {
+      throw new SSLException("Oops! Hostname verification failed!");
+    }
   }
 
-  // BAD: ignored result of HostnameVerifier.verify()
-  public static SSLSocket connectAndOnlyPrintFailureOfHostnameVerification(
+  // 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();
-    boolean failed = verifier.verify(host, socket.getSession());
-    if (failed) {
-      System.out.println("Hostname verification failed");
-    }
-
+    check(verifier.verify(host, socket.getSession()));
     return socket;
   }