CPP: Add query for CWE-297: Improper Validation of Certificate with Host Mismatch

cpp/ql/src/experimental/Security/CWE/CWE-297/ValidationCertificateHostMismatch.cpp

@@ -0,0 +1,13 @@
+
+...
+  SSL_set_hostflags(ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+  if (!SSL_set1_host(ssl, host)) return false;
+  SSL_set_verify(ssl, SSL_VERIFY_PEER, NULL); // GOOD
+...
+  result = SSL_get_verify_result(ssl);
+  if (result == X509_V_OK)
+  {
+    cert = SSL_get_peer_certificate(ssl);
+    if(cert) return true; // BAD
+  }
+...

cpp/ql/src/experimental/Security/CWE/CWE-297/ValidationCertificateHostMismatch.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Lack of certificate name validation allows any valid certificate to be used. And that can lead to disclosure.</p>
+
+</overview>
+
+<example>
+<p>The following example shows working with and without certificate name verification.</p>
+<sample src="ValidationCertificateHostMismatch.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87150715">DRD19. Properly verify server certificate on SSL/TLS - Android - Confluence</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-297/ValidationCertificateHostMismatch.ql

@@ -0,0 +1,141 @@
+/**
+ * @name Validation certificate host mismatch.
+ * @description Lack of certificate name validation allows any valid certificate to be used. And that can lead to disclosure.
+ * @kind problem
+ * @id cpp/validation-certificate-host-mismatch
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-297
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+FunctionCall getParentCall(FunctionCall f1, FunctionCall f2) {
+  result = f1 and f1.getEnclosingFunction() = f2.getEnclosingFunction()
+  or
+  result = getParentCall(f1.getEnclosingFunction().getACallToThisFunction(), f2)
+  or
+  result = getParentCall(f1, f2.getEnclosingFunction().getACallToThisFunction())
+}
+
+predicate checkHostnameOlder1(FunctionCall fc) {
+  exists(FunctionCall fctmp1, FunctionCall fcNode |
+    fcNode = getParentCall(fctmp1, fc) and
+    fctmp1.getTarget().hasName("X509_get_ext_d2i") and
+    (
+      exists(MacroInvocation mi1tmp, MacroInvocation mi2tmp |
+        mi1tmp.getMacroName() = "sk_GENERAL_NAME_num" and
+        mi2tmp.getMacroName() = "sk_GENERAL_NAME_value"
+      )
+      or
+      exists(FunctionCall fctmp2, FunctionCall fctmp3 |
+        fctmp2.getTarget().hasName("sk_GENERAL_NAME_num") and
+        fctmp3.getTarget().hasName("sk_GENERAL_NAME_value")
+      )
+    )
+  )
+}
+
+predicate checkHostnameOlder2(FunctionCall fc) {
+  exists(
+    FunctionCall fctmp1, FunctionCall fctmp2, FunctionCall fctmp3, FunctionCall fctmp4,
+    FunctionCall fcNode
+  |
+    fcNode = getParentCall(fctmp1, fc) and
+    fctmp3 != fctmp4 and
+    globalValueNumber(fctmp1) = globalValueNumber(fctmp3.getArgument(0)) and
+    globalValueNumber(fctmp2) = globalValueNumber(fctmp4.getArgument(0)) and
+    fctmp1.getTarget().hasName("X509_get_subject_name") and
+    fctmp2.getTarget().hasName("X509_get_subject_name") and
+    fctmp3.getTarget().hasName("X509_NAME_get_text_by_NID") and
+    (
+      fctmp4.getTarget().hasName("X509_NAME_get_text_by_NID") or
+      fctmp4.getTarget().hasName("X509_NAME_get_entry")
+    )
+  )
+}
+
+predicate checkHostnameUp110(FunctionCall fc) {
+  exists(FunctionCall fctmp1, FunctionCall fctmp2, FunctionCall fctmp3, FunctionCall fcNode |
+    fcNode = getParentCall(fctmp1, fc) and
+    globalValueNumber(fctmp1.getArgument(0)) = globalValueNumber(fctmp2.getArgument(0)) and
+    globalValueNumber(fctmp2.getArgument(0)) = globalValueNumber(fctmp3.getArgument(0)) and
+    fctmp1.getTarget().hasName("SSL_set_hostflags") and
+    fctmp2.getTarget().hasName("SSL_set1_host") and
+    fctmp3.getTarget().hasName("SSL_set_verify")
+  )
+}
+
+predicate checkHostnameUp102(FunctionCall fc) {
+  exists(
+    FunctionCall fctmp1, FunctionCall fctmp2, FunctionCall fctmp3, FunctionCall fctmp4,
+    FunctionCall fcNode
+  |
+    fcNode = getParentCall(fctmp1, fc) and
+    globalValueNumber(fctmp1.getArgument(0)) = globalValueNumber(fctmp4.getArgument(0)) and
+    globalValueNumber(fctmp1) = globalValueNumber(fctmp2.getArgument(0)) and
+    globalValueNumber(fctmp1) = globalValueNumber(fctmp3.getArgument(0)) and
+    fctmp1.getTarget().hasName("SSL_get0_param") and
+    fctmp2.getTarget().hasName("X509_VERIFY_PARAM_set_hostflags") and
+    fctmp3.getTarget().hasName("X509_VERIFY_PARAM_set1_host") and
+    fctmp4.getTarget().hasName("SSL_set_verify")
+  )
+}
+
+predicate checkHostnameGnuTLS(FunctionCall fc) {
+  exists(FunctionCall functionCheckHost, FunctionCall fcNode |
+    fcNode = getParentCall(functionCheckHost, fc) and
+    (
+      functionCheckHost
+          .getTarget()
+          .hasName(["gnutls_x509_crt_check_hostname", "gnutls_x509_crt_check_hostname2"])
+      or
+      functionCheckHost.getTarget().hasName("gnutls_certificate_verify_peers3") and
+      not functionCheckHost.getArgument(1).getValue() = "0"
+    )
+  )
+}
+
+predicate checkHostnameBOOST1(FunctionCall fc) {
+  exists(FunctionCall functionCallBackForCheck, FunctionCall fcNode |
+    fcNode = getParentCall(functionCallBackForCheck, fc) and
+    functionCallBackForCheck.getTarget().hasName("set_verify_callback")
+  )
+}
+
+predicate checkHostnameBOOST2(FunctionCall fc) {
+  exists(FunctionCall functionCallBackForCheck, FunctionCall fcNode |
+    fcNode = getParentCall(functionCallBackForCheck, fc) and
+    functionCallBackForCheck.getTarget().hasName("set_verify_callback") and
+    not functionCallBackForCheck
+        .getArgument(0)
+        .(FunctionCall)
+        .getTarget()
+        .hasName(["rfc2818_verification", "host_name_verification"])
+  )
+}
+
+from FunctionCall fc
+where
+  not checkHostnameOlder1(fc) and
+  not checkHostnameOlder2(fc) and
+  not checkHostnameUp110(fc) and
+  not checkHostnameUp102(fc) and
+  fc.getTarget().hasName(["SSL_set_verify", "SSL_get_peer_certificate", "SSL_get_verify_result"])
+  or
+  not checkHostnameGnuTLS(fc) and
+  fc.getTarget()
+      .hasName([
+          "gnutls_certificate_verify_peers", "gnutls_certificate_verify_peers2",
+          "gnutls_certificate_verify_peers3"
+        ])
+  or
+  (
+    not checkHostnameBOOST1(fc) or
+    checkHostnameBOOST2(fc)
+  ) and
+  fc.getTarget().hasName("set_verify_mode")
+select fc.getEnclosingFunction(), "You may have missed checking the name of the certificate."

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-297/semmle/tests/ValidationCertificateHostMismatch.expected

@@ -0,0 +1,5 @@
+| test1.cpp:47:5:47:12 | badTest1 | You may have missed checking the name of the certificate. |
+| test1.cpp:53:5:53:12 | badTest2 | You may have missed checking the name of the certificate. |
+| test1.cpp:59:5:59:12 | badTest3 | You may have missed checking the name of the certificate. |
+| test2.cpp:41:6:41:13 | badTest1 | You may have missed checking the name of the certificate. |
+| test.cpp:138:6:138:13 | badTest1 | You may have missed checking the name of the certificate. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-297/semmle/tests/ValidationCertificateHostMismatch.qlref

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

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-297/semmle/tests/test.cpp

@@ -0,0 +1,150 @@
+#define X509_V_OK 0
+#define NULL  0
+#define X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS 1
+#define SSL_VERIFY_PEER 1
+#define STACK_OF(type) struct type
+
+typedef unsigned long size_t;
+
+struct SSL {};
+struct X509_STORE_CTX {};
+struct X509_VERIFY_PARAM {};
+struct X509 {};
+struct X509_NAME {};
+struct X509_NAME_ENTRY {};
+
+struct ASN1_STRING {};
+
+struct GENERAL_NAME 
+{
+ASN1_STRING *d_dNSName;
+};
+
+int cmpNames(char* name1,char* name2);
+char* calloc( size_t num, size_t size );
+size_t strlen( char * buf);
+
+X509 *SSL_get_peer_certificate(const SSL *ssl);
+int SSL_get_verify_result(const SSL *ssl);
+
+int verify_callback(int a, X509_STORE_CTX *b);
+X509_VERIFY_PARAM *SSL_get0_param(SSL *ssl);
+void X509_VERIFY_PARAM_set_hostflags(X509_VERIFY_PARAM *param,unsigned int flags);
+int X509_VERIFY_PARAM_set1_host(X509_VERIFY_PARAM *param,char *name, size_t namelen);
+void SSL_set_verify(SSL *s, int mode,int (*verify_callback)(int, X509_STORE_CTX *));
+
+void SSL_set_hostflags(SSL *s, unsigned int flags);
+int SSL_set1_host(SSL *s, const char *hostname);
+
+GENERAL_NAME * X509_get_ext_d2i(const X509 *x, int nid, int *crit, int *idx);
+int X509_NAME_get_text_by_NID(X509_NAME *name, int nid, char *buf,int len);
+X509_NAME *X509_get_subject_name(const X509 *x);
+int X509_NAME_get_index_by_NID(X509_NAME *name,int nid,int lastpos);
+X509_NAME_ENTRY *X509_NAME_get_entry(X509_NAME *name, int loc);
+ASN1_STRING *X509_NAME_ENTRY_get_data(const X509_NAME_ENTRY *ne);
+char * ASN1_STRING_data(ASN1_STRING *x);
+
+int SKM_sk_num(struct GENERAL_NAME * a);
+#define 	sk_GENERAL_NAME_num(st)   SKM_sk_num((st))
+GENERAL_NAME* SKM_sk_value(struct GENERAL_NAME * a,int b);
+#define sk_GENERAL_NAME_value(st, i)   SKM_sk_value((st), (i))
+bool goodTest1(SSL *ssl,char * host,int nid) // GOOD
+{
+  X509 *cert;
+  int result,count,i;
+  char *data;
+  STACK_OF(GENERAL_NAME) *names = NULL;
+  GENERAL_NAME *altname;
+
+  result = SSL_get_verify_result(ssl);
+  if (result == X509_V_OK)
+  {
+    cert = SSL_get_peer_certificate(ssl);
+    names = X509_get_ext_d2i (cert, nid, NULL, NULL);
+    if(names==NULL) return false;
+    count = sk_GENERAL_NAME_num(names);
+    for(i=0;i<count;i++)
+    {
+      altname = sk_GENERAL_NAME_value (names, i);
+      data = ASN1_STRING_data (altname->d_dNSName);
+      if(cmpNames(host,data)==0) return true;
+    }
+    return false;
+  }
+  return false;
+}
+bool goodTest2(SSL *ssl,char * host,int nid) // GOOD
+{
+  X509 *cert;
+  int result,len,i;
+  char *data;
+  STACK_OF(GENERAL_NAME) *names = NULL;
+  GENERAL_NAME *altname;
+  X509_NAME *name;
+
+  result = SSL_get_verify_result(ssl);
+  if (result == X509_V_OK)
+  {
+    cert = SSL_get_peer_certificate(ssl);
+    name = X509_get_subject_name (cert);
+    if (name == NULL) return false;
+    len = X509_NAME_get_text_by_NID (name, nid, NULL, 0);
+    if (len < 0) return false;
+    data = calloc (len + 1, 1);
+    if (data == NULL) return false;
+    X509_NAME_get_text_by_NID (name, nid, data, len + 1);
+    if(cmpNames(host,data)==0) return true;
+  }
+  return false;
+}
+
+bool goodTest3(SSL *ssl,char * host,int nid) // GOOD
+{
+  X509 *cert;
+  int result,len,i;
+  char *data;
+  STACK_OF(GENERAL_NAME) *names = NULL;
+  GENERAL_NAME *altname;
+  X509_NAME_ENTRY * centry;
+  result = SSL_get_verify_result(ssl);
+  if (result == X509_V_OK)
+  {
+    cert = SSL_get_peer_certificate(ssl);
+    len = X509_NAME_get_text_by_NID (X509_get_subject_name (cert), nid, NULL, 0);
+    if (len < 0) return false;
+    centry = X509_NAME_get_entry(X509_get_subject_name(cert), len);
+    if(centry == NULL) return false;
+    data = (char *) ASN1_STRING_data(X509_NAME_ENTRY_get_data(centry));
+    if(cmpNames(host,data)==0) return true;
+  }
+  return false;
+}
+
+bool goodTest4(SSL *ssl,char * host) // GOOD
+{
+  SSL_set_hostflags(ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+  if (!SSL_set1_host(ssl, host)) return false;
+  SSL_set_verify(ssl, SSL_VERIFY_PEER, NULL);
+  return true;
+}
+bool goodTest5(SSL *ssl,char * host) // GOOD
+{
+  X509_VERIFY_PARAM *param = SSL_get0_param(ssl);
+  X509_VERIFY_PARAM_set_hostflags(param,X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+  if (!X509_VERIFY_PARAM_set1_host(param, host,strlen(host))) return false;
+  SSL_set_verify(ssl, SSL_VERIFY_PEER, NULL);
+  return true;
+}
+bool badTest1(SSL *ssl) // BAD :no hostname verification in certificate
+{
+  X509 *cert;
+  int result;
+
+  result = SSL_get_verify_result(ssl);
+  if (result == X509_V_OK)
+  {
+    cert = SSL_get_peer_certificate(ssl);
+    if(cert) return true;
+  }
+  return false;
+}