C++: Reuse bounded predicate in TaintedAllocationSize query

[paldepind]

Reuse the bounded predicate in the TaintedAllocationSize query.

[geoffw0]

Changes LGTM.

We looked at some MRVA results and were happy with the differences. We should review the DCA results as well before merging.

And since this change does affect results, add a change note.

[geoffw0]

Note that it's a pretty common pattern to have three cases to think about (1) a thing holds (2) the thing doesn't hold and (3) we weren't able to determine either. If we had another predicate, convertedExprNeverOverflows, this would not be the inverse of convertedExprMightOverflow (due to case 3) and the choice between using convertedExprNeverOverflows or not convertedExprMightOverflow as we do here would be meaningful.

[geoffw0]

Happy with this, when you've finished reviewing a sample of those DCA differences.

[paldepind]

I've taken a look at the results in the DCA report. With this PR there are 11 less query results.

One of the disappearing results is in git. The added barrier is c & 15 which is correct as that certainly bounds the input.

8 of the disappearing results are in vim. After looking at them, they all go through the same function with a bitwise & barrier. Here the mask, CT_CELL_MASK, is 7, so this also effectively bounds the value (and even before that the paths are very long (60-120) and kinda questionable).

The last 2 results are in openjdk. Here the paths are short (10 steps). The source reads a file which ends up tainting an allocation. In between some bit manipulation goes on, apparently to extract the right bits from a header. I think this could be a true positive for the purpose of the query, but it's clearly deliberate that those bits in the header should end up affecting the allocation.

I think the results seem fine. I think one future approach that could increase precision would be to consider that actual values used in the arithmetic operations (&, /, %, etc.) and only accept barriers where the operation actually reduces the size significantly (such as % sizeof(char) but not / sizeof(char)).

[geoffw0]

Looks great, thanks for addressing this!

[paldepind]

Fixed some conflicts after merging the other PR related to TaintedAllocationSize. Should be ready for final review now :)

[geoffw0]

LGTM.

[intrigus-lgtm]

Why is this example "GOOD", but the one from lines 132-136 isn't?
That is, I can't see a real difference here:
https://godbolt.org/z/bPWaoPMYY

[intrigus-lgtm]

I guess my actual question is: how is modulo supposed to bound the input here?

[paldepind]

That's a good point. It does not 😅. As it stands it's actually an example of a false negative. What I had in my mind was an example like this:

int size = atoi(getenv("USER"));
int size2 = size % 100;
malloc(size2 * sizeof(int));

[paldepind]

Thanks for pointing this out @intrigus-lgtm. It's fixed now by #17269.