create new branchihsinme-patch-88 in fork

Author: ihsinme

cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.cpp

@@ -0,0 +1,11 @@
+...
+SSL_shutdown(ssl); 
+SSL_shutdown(ssl); // BAD
+...
+    switch ((ret = SSL_shutdown(ssl))) {
+    case 1:
+      break;
+    case 0:
+      ERR_clear_error();
+      if (-1 != (ret = SSL_shutdown(ssl))) break; // GOOD
+...

cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Incorrect closing of the connection leads to the creation of different states for the server and client, which can be exploited by an attacker.</p>
+
+</overview>
+
+<example>
+<p>The following example shows the incorrect and correct usage of function SSL_shutdown.</p>
+<sample src="DangerousUseSSL_shutdown.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP12-C.+Do+not+ignore+values+returned+by+functions">EXP12-C. Do not ignore values returned by functions - SEI CERT C Coding Standard - Confluence</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql

@@ -0,0 +1,33 @@
+/**
+ * @name Dangerous use SSL_shutdown.
+ * @description Incorrect closing of the connection leads to the creation of different states for the server and client, which can be exploited by an attacker.
+ * @kind problem
+ * @id cpp/dangerous-use-of-ssl_shutdown
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-670
+ */
+
+import cpp
+import semmle.code.cpp.commons.Exclusions
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+from FunctionCall fc, FunctionCall fc1
+where
+  fc != fc1 and
+  fc.getASuccessor+() = fc1 and
+  fc.getTarget().hasName("SSL_shutdown") and
+  fc1.getTarget().hasName("SSL_shutdown") and
+  fc1 instanceof ExprInVoidContext and
+  (
+    globalValueNumber(fc.getArgument(0)) = globalValueNumber(fc1.getArgument(0)) or
+    fc.getArgument(0).(VariableAccess).getTarget() = fc1.getArgument(0).(VariableAccess).getTarget()
+  ) and
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasName("SSL_free") and
+    fc.getASuccessor+() = fctmp and
+    fctmp.getASuccessor+() = fc1
+  )
+select fc, "You need to handle the return value SSL_shutdown"

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.expected

@@ -0,0 +1,2 @@
+| test.cpp:45:20:45:31 | call to SSL_shutdown | You need to handle the return value SSL_shutdown |
+| test.cpp:61:11:61:22 | call to SSL_shutdown | You need to handle the return value SSL_shutdown |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql

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

@@ -0,0 +1,75 @@
+// it's not exact, but it's enough for an example
+typedef int SSL;
+
+
+int SSL_shutdown(SSL *ssl);
+int SSL_get_error(const SSL *ssl, int ret);
+void ERR_clear_error(void);
+void print_error(char *buff,int code);
+
+int gootTest1(SSL *ssl)
+{
+  int ret;
+    switch ((ret = SSL_shutdown(ssl))) {
+    case 1:
+      break;
+    case 0:
+      ERR_clear_error();
+      if ((ret = SSL_shutdown(ssl)) == 1) break; // GOOD
+    default:
+      print_error("error shutdown",
+        SSL_get_error(ssl, ret));
+      return -1;
+    }
+ return 0;
+}
+int gootTest2(SSL *ssl)
+{
+  int ret;
+    switch ((ret = SSL_shutdown(ssl))) {
+    case 1:
+      break;
+    case 0:
+      ERR_clear_error();
+      if (-1 != (ret = SSL_shutdown(ssl))) break; // GOOD
+    default:
+      print_error("error shutdown",
+        SSL_get_error(ssl, ret));
+      return -1;
+    }
+ return 0;
+}
+int badTest1(SSL *ssl)
+{
+  int ret;
+    switch ((ret = SSL_shutdown(ssl))) {
+    case 1:
+      break;
+    case 0:
+      SSL_shutdown(ssl); // BAD
+      break;
+    default:      
+      print_error("error shutdown",
+        SSL_get_error(ssl, ret));
+      return -1;
+    }
+ return 0;
+}
+int badTest2(SSL *ssl)
+{
+  int ret;
+    ret = SSL_shutdown(ssl);
+    switch (ret) {
+    case 1:
+      break;
+    case 0:
+      SSL_shutdown(ssl); // BAD
+      break;
+    default:
+      print_error("error shutdown",
+        SSL_get_error(ssl, ret));
+      return -1;
+    }
+ return 0;
+}
+