Skip to content

Commit 57d1035

Browse files
author
Porcupiney Hairs
committed
Include changes from review
1 parent e768e2e commit 57d1035

File tree

5 files changed

+21
-16
lines changed

5 files changed

+21
-16
lines changed
Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
22
<qhelp>
33
<overview>
4-
Disabling verification of the SSL certificate allows man-in-the-middle attacks. Disabling the
5-
peer or the host's certificate verification makes the SSL communication insecure. Just having
6-
encryption on a transfer is not enough as you cannot be sure that you are communicating with the
7-
correct end-point.
4+
Disabling verification of the SSL certificate allows man-in-the-middle attacks.
5+
A SSL connection is vulnerable to man-in-the-middle attacks if the certification is not checked
6+
properly.
7+
If the peer or the host's certificate verification is not verified, the underlying SSL
8+
communication is insecure.
89
</overview>
910
<recommendation>
1011
It is recommended that all communications be done post verification of the host as well as the
@@ -21,10 +22,12 @@
2122
<references>
2223
<li> Curl Documentation:<a href="https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html">
2324
CURLOPT_SSL_VERIFYHOST</a></li>
24-
<li> Curl Documentation:<a href="https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html">
25-
CURLOPT_SSL_VERIFYHOST</a></li>
26-
<li> Related CVE: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-33684"> CVE-2022-33684</a></li>
27-
<li> Related CVE: <a href="https://huntr.com/bounties/42325662-6329-4e04-875a-49e2f5d69f78">
28-
`openframeworks/openframeworks`</a></li>
25+
<li> Curl Documentation:<a href="https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYPEER.html">
26+
CURLOPT_SSL_VERIFYPEER</a></li>
27+
<li> Related CVE: <a href="https://github.com/advisories/GHSA-5r3h-c3r7-9w4h"> CVE-2022-33684</a></li>
28+
<li> Related security advisory: <a
29+
href="https://huntr.com/bounties/42325662-6329-4e04-875a-49e2f5d69f78">
30+
<code>openframeworks/openframeworks</code>
31+
</a></li>
2932
</references>
3033
</qhelp>

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import semmle.code.cpp.dataflow.new.TaintTracking
1515
private class CurlSetOptCall extends FunctionCall {
1616
CurlSetOptCall() {
1717
exists(FunctionCall fc, Function f |
18-
f.hasGlobalName("curl_easy_setopt") and
18+
f.hasGlobalOrStdName("curl_easy_setopt") and
1919
fc.getTarget() = f
2020
|
2121
this = fc
@@ -34,6 +34,7 @@ private class CurlVerificationConstant extends EnumConstantAccess {
3434

3535
from CurlSetOptCall c
3636
where
37-
c.getArgument(1) = any(CurlVerificationConstant v) and
37+
c.getArgument(1) = any(CurlVerificationConstant v)
38+
and
3839
c.getArgument(2).getValue() = "0"
3940
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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ void bad(void) {
44
std::unique_ptr<CURL, void(*)(CURL*)>(curl_easy_init(), curl_easy_cleanup);
55
curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 0);
66
curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 0);
7-
curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
8-
curl_easy_perform(curl.get());
7+
curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
8+
curl_easy_perform(curl.get());
99
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ void good(void) {
44
std::unique_ptr<CURL, void(*)(CURL*)>(curl_easy_init(), curl_easy_cleanup);
55
curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYPEER, 2);
66
curl_easy_setopt(curl.get(), CURLOPT_SSL_VERIFYHOST, 2);
7-
curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
8-
curl_easy_perform(curl.get());
7+
curl_easy_setopt(curl.get(), CURLOPT_URL, host.c_str());
8+
curl_easy_perform(curl.get());
99
}
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
a
1+
| CurlSSL.cpp:25:2:25:17 | call to curl_easy_setopt | This call disables Secure Socket Layer and could potentially lead to MITM attacks |
2+
| CurlSSL.cpp:26:2:26:17 | call to curl_easy_setopt | This call disables Secure Socket Layer and could potentially lead to MITM attacks |

0 commit comments

Comments
 (0)