C++: New query cpp/potential-system-data-exposure

[geoffw0]

This is a new query cpp/potential-system-data-exposure closely related to the existing cpp/system-data-exposure, both providing coverage of CWE-497. Roughly speaking, each result of the old cpp/system-data-exposure is now either:

• still a result of cpp/system-data-exposure, if the information is exposed to a remote sink (this change was made in a previous PR).
• a result of this new query cpp/potential-system-data-exposure, if the above does not hold but the information exposed is considered especially sensitive (e.g. a password being output to stdout).
• not a result any more (the old query used to be very noisy).

This part is likely to remain @precision medium, because some projects will consider standard output and similar channels to be safe from attackers. cpp/system-data-exposure is likely to be upgraded to @precision high, as most of those results will carry some real risk.

Note that the diff is larger than the true number of changes because:

• I've moved some existing QL code into a SystemData.qll library, shared by both queries.
• I've migrated most of our internal tests for CWE-497 to the public repo (there's an internal PR removing them there).

---

LGTM results: https://lgtm.com/query/7993974152184061577/

• results look reasonable to me
• there are a few cases being picked up where we read into one field of a struct and output from another; this isn't ideal, but in most cases the result is reasonable regardless.

I've also started a DCA run.

[MathiasVP]

This looks great! Here are my initial comments.

[geoffw0]

DCA shows:

• 9 results for the new query; all are in kamailio__kamailio, matching results we'd already found via LGTM.
• some performance wobbles, including analysis 3% slower overall; I don't believe this is significant.
• cpp/potential-system-data-exposure is probably in the slower half of queries for performance, but far from the slowest, I don't think its causing expensive re-evaluations or anything like that.

[geoffw0]

I think this is PR is ready, apart from merge conflicts and submodule stuff which I'll address tomorrow.

[MathiasVP]

LGTM! Do we want a docs review of these changes?

[geoffw0]

Do we want a docs review of these changes?

Yeah, for the two .qhelp files.

@github/docs-content-codeql

[docs-bot]

📚 Thanks for the docs ping! 🛎️ This was added to our docs first-responder project board. A team member will be along shortly to respond. To request changes to the docs you can also open a CodeQL docs issue.

[skedwards88]

Docs first responder here 👋 I've added this PR to our board for writer review.

[mchammer01]

Not sure I'll get to reviewing this today. If not, I'll look at this tomorrow 👍🏻

[mchammer01]

@geoffw0 - LGTM ✨
Approving this so that I don't block this work.
I've left a few suggestions and comments for your consideration. Feel free to ignore them if you don't agree 🙂

Also, I didn't see a change note about the new query, but I'm not sure whether all CodeQL teams are adding change notes for new queries.

[mchammer01]

I'd make the same changes to to this qhelp file that I suggested for the other qhelp file adversary -> malicious user, remove the to in may help an adversary to learn about, and possibly find a synonym for technologies.
I am a bit confused because the content for both seems to be exactly the same 😕

[geoffw0]

Done.

We've essentially forked one query into two variations here, and ended up with two versions of the qhelp as well. There are slight differences reflecting the difference in focus between the two queries, most of the text remains identical.

[mchammer01]

Thanks for explaining, it now makes more sense to me 🙂

[MathiasVP]

Also, I didn't see a change note about the new query, but I'm not sure whether all CodeQL teams are adding change notes for new queries.

All language teams should add a change-note for a new query. It's also done in this PR. See this commit.

[geoffw0]

I think I've addressed all concerns / suggestions now.

[MathiasVP]

LGTM!

[mchammer01]

@MathiasVP - thanks for pointing the change note to me, I didn't see it at all 😞
LGTM ✨