Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CPP: Disabled SSL certificate verification #16811

Merged
merged 4 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSL.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
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.
</overview>
<recommendation>
It is recommended that all communications be done post verification of the host as well as the
peer.
</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">
<code>openframeworks/openframeworks</code>
</a></li>
</references>
</qhelp>
40 changes: 40 additions & 0 deletions cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSL.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* @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"
9 changes: 9 additions & 0 deletions cpp/ql/src/experimental/Security/CWE/CWE-295/CurlSSLBad.cpp
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
@@ -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();
}

Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE/CWE-295/CurlSSL.ql
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
Loading