CPP: Disabled SSL certificate verification

[porcupineyhairs]

Disable SSL certificate verification can expose the communication to MITM attacks.

This PR adds a query to detect the same. This also include the tests and qhelp for the same.

[ghsecuritylab]

Hello porcupineyhairs 👋
You have submitted this pull request as a bug bounty report in the github/securitylab repository and therefore this pull request has been put into draft state to give time for the GitHub Security Lab to assess the PR. When GitHub Security Lab has finished assessing your pull request, it will be marked automatically as Ready for review. Until then, please don't change the draft state.

In the meantime, feel free to make changes to the pull request. If you'd like to maximize payout for your this and future submissions, here are a few general guidelines, that we might take into consideration when reviewing a submission.

• the submission models widely-used frameworks/libraries
• the vulnerability modeled in the submission is impactful
• the submission finds new true positive vulnerabilities
• the submission finds very few false positives
• code in the submission is easy to read and will be easy to maintain
• documentation is written clearly, highlighting the impact of the issue it finds and is written without grammatical or other errors. The code samples clearly show the vulnerability
• the submission includes tests, change note etc.

Please note that these are guidelines, not rules. Since we have a lot of different types of submissions, the guidelines might vary for each submission.

Happy hacking!

[geoffw0]

Query looks good to me - just a few suggestions around the docs and test (the docs don't need to be perfect for experimental though). I also found some results in the wild, and all of those results looked great!

[porcupineyhairs]

@geoffw0 I have included the changes from the review. However, for some reason, the tests don't work anymore. I am unable to diagnose what's wrong here. Can you please take a look?

[porcupineyhairs]

@geoffw0 Changes done!

[geoffw0]

LGTM. We can merge this when the CI checks pass.

[github-actions]

QHelp previews:

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

Disabled certifcate verification

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.

Recommendation

It is recommended that all communications be done post verification of the host as well as the peer.

Example

The following snippet disables certification verification by setting the value of CURLOPT_SSL_VERIFYHOST and CURLOPT_SSL_VERIFYHOST to 0:

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

This is bad as the certificates are not verified any more. This can be easily fixed by setting the values of the options to 2.

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

References

• Curl Documentation: CURLOPT_SSL_VERIFYHOST
• Curl Documentation: CURLOPT_SSL_VERIFYPEER
• Related CVE: CVE-2022-33684
• Related security advisory: openframeworks/openframeworks
• Common Weakness Enumeration: CWE-295.

[geoffw0]

Looks like you need to:

• autoformat CurlSSL.ql
• fix the qhelp; I think you're just missing <p> / </p> tags around your text blocks.

Let me know if you need any help with these things.

[porcupineyhairs]

@geoffw0 Changes done. PTAL.

[geoffw0]

LGTM. 👍