Merge pull request #16811 from porcupineyhairs/curlssl

CPP: Disabled SSL certificate verification

Author: jketema

cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSL.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Disabling verification of the SSL certificate allows man-in-the-middle attacks. A SSL
+      connection is vulnerable to man-in-the-middle attacks if the certification is not checked
+      properly. If the peer or the host's certificate verification is not verified, the underlying
+      SSL communication is insecure.</p>
+  </overview>
+  <recommendation>
+    <p>It is recommended that all communications be done post verification of the host as well as
+      the
+      peer.</p>
+  </recommendation>
+  <example>
+    <p>The following snippet disables certification verification by setting the value of <code>
+      CURLOPT_SSL_VERIFYHOST</code> and <code>CURLOPT_SSL_VERIFYHOST</code> to <code>0</code>:</p>
+    <sample src="CurlSSLBad.cpp" />
+    <p>This is bad as the certificates are not verified any more. This can be easily fixed by
+      setting the values of the options to <code>2</code>. </p>
+    <sample src="CurlSSLGood.cpp" />
+  </example>
+  <references>
+    <li> Curl Documentation:<a href="https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html">
+      CURLOPT_SSL_VERIFYHOST</a></li>
+    <li> Curl Documentation:<a href="https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYPEER.html">
+      CURLOPT_SSL_VERIFYPEER</a></li>
+    <li> Related CVE: <a href="https://github.com/advisories/GHSA-5r3h-c3r7-9w4h"> CVE-2022-33684</a></li>
+    <li> Related security advisory: <a
+        href="https://huntr.com/bounties/42325662-6329-4e04-875a-49e2f5d69f78">
+      openframeworks/openframeworks
+      </a></li>
+  </references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSL.ql

@@ -0,0 +1,39 @@
+/**
+ * @name Disabled certifcate verification
+ * @description Disabling SSL certificate verification of host or peer could expose the communication to man-in-the-middle(MITM) attacks.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/curl-disabled-ssl
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.TaintTracking
+
+/** Models the `curl_easy_setopt` function call */
+private class CurlSetOptCall extends FunctionCall {
+  CurlSetOptCall() {
+    exists(FunctionCall fc, Function f |
+      f.hasGlobalOrStdName("curl_easy_setopt") and
+      fc.getTarget() = f
+    |
+      this = fc
+    )
+  }
+}
+
+/** Models an access to any enum constant which could affect SSL verification */
+private class CurlVerificationConstant extends EnumConstantAccess {
+  CurlVerificationConstant() {
+    exists(EnumConstant e | e.getName() = ["CURLOPT_SSL_VERIFYHOST", "CURLOPT_SSL_VERIFYPEER"] |
+      e.getAnAccess() = this
+    )
+  }
+}
+
+from CurlSetOptCall c
+where
+  c.getArgument(1) = any(CurlVerificationConstant v) and
+  c.getArgument(2).getValue() = "0"
+select c, "This call disables Secure Socket Layer and could potentially lead to MITM attacks"

cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSLBad.cpp

@@ -0,0 +1,9 @@
+string host = "codeql.com"
+void bad(void) {
+	std::unique_ptr<CURL, void(*)(CURL*)> curl =
+		std::unique_ptr<CURL, void(*)(CURL*)>(curl_easy_init(), curl_easy_cleanup);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 0);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 0); 
+  	curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
+  	curl_easy_perform(curl.get());
+}

cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSLGood.cpp

@@ -0,0 +1,9 @@
+string host = "codeql.com"
+void good(void) {
+	std::unique_ptr<CURL, void(*)(CURL*)> curl =
+		std::unique_ptr<CURL, void(*)(CURL*)>(curl_easy_init(), curl_easy_cleanup);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 2);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 2); 
+  	curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
+  	curl_easy_perform(curl.get());
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-295/CurlSSL.cpp

@@ -0,0 +1,43 @@
+#include "../../../../../library-tests/string_concat/stl.h"
+
+namespace std{
+	struct CURL {};
+	typedef CURL curl;
+	enum curl_constant{
+		CURLOPT_URL,
+		CURLOPT_SSL_VERIFYHOST,
+		CURLOPT_SSL_VERIFYPEER
+	}; 
+
+	CURL *curl_easy_init();
+	void curl_easy_cleanup(CURL *handle);
+	void curl_easy_perform(CURL *handle);
+	void curl_easy_setopt(CURL *handle, curl_constant param, int p);
+	void curl_easy_setopt(CURL *handle, curl_constant param, char* p);
+}
+
+
+using namespace std;
+char host[] = "codeql.com";
+
+void bad(void) {
+	std::unique_ptr<CURL> curl = std::unique_ptr<CURL>(curl_easy_init());
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 0);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 0); 
+  	curl_easy_setopt(curl.get(), CURLOPT_URL, host);
+  	curl_easy_perform(curl.get());
+}
+
+void good(void) {
+	std::unique_ptr<CURL> curl = std::unique_ptr<CURL>(curl_easy_init());
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 2);
+	curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 2); 
+  	curl_easy_setopt(curl.get(), CURLOPT_URL, host);
+  	curl_easy_perform(curl.get());
+}
+
+int main(int c, char** argv){
+	bad();
+  	good();
+}
+

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-295/CurlSSL.expected

@@ -0,0 +1,2 @@
+| CurlSSL.cpp:25:2:25:17 | call to curl_easy_setopt | This call disables Secure Socket Layer and could potentially lead to MITM attacks |
+| CurlSSL.cpp:26:2:26:17 | call to curl_easy_setopt | This call disables Secure Socket Layer and could potentially lead to MITM attacks |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-295/CurlSSL.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-295/CurlSSL.ql