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 1 commit
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
30 changes: 30 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,30 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
Disabling verification of the SSL certificate allows man-in-the-middle attacks. Disabling the
peer or the host's certificate verification makes the SSL communication insecure. Just having
encryption on a transfer is not enough as you cannot be sure that you are communicating with the
correct end-point.
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
</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_VERIFYHOST.html">
CURLOPT_SSL_VERIFYHOST</a></li>
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
<li> Related CVE: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-33684"> CVE-2022-33684</a></li>
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
<li> Related CVE: <a href="https://huntr.com/bounties/42325662-6329-4e04-875a-49e2f5d69f78">
`openframeworks/openframeworks`</a></li>
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
</references>
</qhelp>
39 changes: 39 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,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.hasGlobalName("curl_easy_setopt") and
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
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());
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
}
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());
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#include<iostream>
#include<memory>

#include<curl/curl.h>
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved

using namespace std;

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());
}

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());
}

int main(int c, char** argv){
bad();
good();
}
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
semmle-extractor-options: command='g++ -lcurl'
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
Loading