C++: Refine Node.asDefinition

[MathiasVP]

In #18942 we're slightly changing how we handle uncertain writes. For example, consider this example:

int buffer[10];
write_tainted_data(buffer); // 1
buffer[i] = '\0'; // 2
sink(*buffer); // 3

On main we get dataflow from // 1 to // 3 by realizing that // 2 is an uncertain write (i.e., it doesn't overwrite the entire buffer), and so we insert a direct def-use step from // 1 to // 3 here.

This has always been a bit of a hack since this can create a quadratic number of def-use edges. Luckily, the DataFlowIntegration module that Schack is porting to C++ in #18942 does flow through uncertain writes slightly differently. In the above example, it adds a step from // 1 to // 2 and (because //2 is an uncertain definition) another step from // 2 to // 3. This gets rid of the quadratic number of steps 🎉

However, we have some queries that assume that, if we reach a StoreInstruction then we'll continue with that def-use flow. For example here we place a barrier when writing to a non-char pointer type since we don't want to track flow to arithmetic values in that query.

Now that we have other flows out of that StoreInstruction this no longer holds true. So this PR adds a column to asDefinition so that it's possible to check whether or not the definition overwrites the entire buffer. I also added convenience predicates asCertainDefinition and asUncertainDefinition to avoid memorizing what that boolean means, and changed all our queries to use the new API.

New results

We have three new results:

• The openjdk-jdk result is a new TP. Previously, we were marking this assignment as a barrier, and we obviusly don't want to do that as this is basically a strcpy-like function. By only marking certain writes as barriers this is no longer considered a barrier for the query.
• The vim__vim result also looks TP-ish. On main we place a barrier on this assignment, and now we don't do that since the write only partially overwrites the buffer. As far as I can tell this write sits inside a very large loop that iterators over some tainted data, and so we write each character individually into format (i.e., we grab each character here). So I thinks this result is genuine as well.
• The new neovim__neovim result is identical to the vim__vim result

[Copilot]

Copilot wasn't able to review any files in this pull request.

Files not reviewed (6)

• cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll: Language not supported
• cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql: Language not supported
• cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql: Language not supported
• cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql: Language not supported
• cpp/ql/src/Security/CWE/CWE-170/ImproperNullTerminationTainted.ql: Language not supported
• cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql: Language not supported

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

[jketema]

Bit of nitpicking below, otherwise LGTM.