C++: Fix missing results in cpp/unbounded-write

[MathiasVP]

I had missed this issue in #14669.

The problem was that every sink was marked as an out barrier. I did this to avoid path duplication when the outgoing argument was marked as a sink to continue to flag up cases such as:

char buffer[1024];
gets(buffer);

However, marking all sinks are out barriers was slightly too aggressive (as seen in the first commit). This is because the query also flags up writes fields when the qualifier of the field is tainted. That is, this is also flagged up:

S* s = tainted_struct();
strcpy(buffer, s->field);

so both struct and s->field are marked as sinks. But since we mark every sink as an out barrier we'd mark s as a barrier, and thus we'd never get a to the read step required to catch something like:

S* s;
s->field = tainted();
strcpy(buffer, s->field);

This PR fixes the problem by only marking gets (and friends) functions as out barriers.

[jketema]

I'm happy with these changes if DCA is happy.

I'm wondering if we should also do a MRVA experiment comparing the results before and after?

[MathiasVP]

I'm wondering if we should also do a MRVA experiment comparing the results before and after?

Yeah, I can start such a run as well 👍

[jketema]

With the latest accepted test changes, do I see correctly that there's some result duplication now? Or am I reading that totally wrong?

[MathiasVP]

You're reading that correctly. The problem happens in a test that looks like:

int main(int argc, char* argv[]) {
  // ...
  sprintf(buffer100, "%s", argv[0]);
  scanf("%s", buffer100);
}

where we now report a path from argv -> argv[0] -> buffer100 -> buffer100 and a path with just buffer100. This is because we no longer block the flow out of the sink at the sprintf.

Thinking more about this, I think a better way to fix this is to simply block flow out of the sinks that are not qualifiers. i.e., don't block flow out of s in

S* s = tainted_struct();
strcpy(buffer, s->field);

but block flow out of s in:

char* s = ...;
strcpy(buffer, s);

[MathiasVP]

I've done ^ in 967bbbc. Let me run a new DCA and MRVA run and see what happens now 🤞

[MathiasVP]

DCA LGTM! The new results are all field-related (as expected), and were present in the old DTT-based query