From 09cd1681970b3a0a2f6ab77a6dbba73399c991c8 Mon Sep 17 00:00:00 2001
From: ihsinme <ihsinme@github.com>
Date: Mon, 9 May 2022 13:05:06 +0000
Subject: [PATCH 1/3] create new branchihsinme-patch-88 in fork

---
 .../CWE/CWE-670/DangerousUseSSL_shutdown.cpp  | 11 +++
 .../CWE-670/DangerousUseSSL_shutdown.qhelp    | 23 ++++++
 .../CWE/CWE-670/DangerousUseSSL_shutdown.ql   | 33 ++++++++
 .../tests/DangerousUseSSL_shutdown.expected   |  2 +
 .../tests/DangerousUseSSL_shutdown.qlref      |  1 +
 .../CWE/CWE-670/semmle/tests/test.cpp         | 75 +++++++++++++++++++
 6 files changed, 145 insertions(+)
 create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.cpp
 create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp
 create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql
 create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.expected
 create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.qlref
 create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/test.cpp

diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.cpp
new file mode 100644
index 00000000000..291cbc8edca
--- /dev/null
+++ b/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
+...
diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp
new file mode 100644
index 00000000000..e093e05a10f
--- /dev/null
+++ b/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>
diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql
new file mode 100644
index 00000000000..548fc0d3748
--- /dev/null
+++ b/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"
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.expected
new file mode 100644
index 00000000000..bb3d9157148
--- /dev/null
+++ b/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 |
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/DangerousUseSSL_shutdown.qlref
new file mode 100644
index 00000000000..0c2096f68ff
--- /dev/null
+++ b/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
diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-670/semmle/tests/test.cpp
new file mode 100644
index 00000000000..9ebe1cc10a5
--- /dev/null
+++ b/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;
+}
+

From f6ab338a166404c6cb9585014e9967cfbef6438a Mon Sep 17 00:00:00 2001
From: ihsinme <ihsinme@gmail.com>
Date: Sun, 15 May 2022 12:26:05 +0300
Subject: [PATCH 2/3] Update DangerousUseSSL_shutdown.qhelp

---
 .../Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp       | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp
index e093e05a10f..bc3f8fcc875 100644
--- a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp
+++ b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.qhelp
@@ -18,6 +18,10 @@
   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>
+<li>
+  Openssl.org:
+  <a href="https://www.openssl.org/docs/man3.0/man3/SSL_shutdown.html">SSL_shutdown - shut down a TLS/SSL connection</a>.
+</li>
 
 </references>
 </qhelp>

From 1a375ec6532b446b208c6396eacaa39621167b64 Mon Sep 17 00:00:00 2001
From: ihsinme <ihsinme@gmail.com>
Date: Mon, 16 May 2022 19:55:33 +0300
Subject: [PATCH 3/3] Update
 cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
---
 .../Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql            | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql
index 548fc0d3748..d608fd5a7ed 100644
--- a/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql
+++ b/cpp/ql/src/experimental/Security/CWE/CWE-670/DangerousUseSSL_shutdown.ql
@@ -2,7 +2,7 @@
  * @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
+ * @id cpp/dangerous-use-of-ssl-shutdown
  * @problem.severity warning
  * @precision medium
  * @tags correctness