C++: Tweak the `bounded` barrier

[paldepind]

trafficstars

The cpp/uncontrolled-allocation-size query has some false negatives, such as this one.

To improve this, @geoffw0 suggested a change to how bounded (which servers as a barrier for this query) works. In an expression like a % 10 the barrier should be put on the whole expression and not on a as is done now. This fixes aforementioned false negative (and potentially others as well).

Note that since bounded is also used in two other queries well have to see to what extend this change impacts them. I'm running MRVA on cpp/uncontrolled-arithmetic and for the top 600 repos I'm seeing 5 results. I don't know what the diff is yet as MRVA is still running.

[paldepind]

In the tests we gained one true positive for cpp/uncontrolled-allocation-size and gained a false positive for cpp/uncontrolled-arithmetic.

When running MRVA on the the top 1000 repositories for cpp/uncontrolled-arithmetic I'm not seeing any difference.

Looking at the DCA we lost one result from cpp/uncontrolled-allocation-size in systemd here. That was a bit surprising to me as the original motivation was to get more results. I think the barrier is on the line m += m / line_break, the tainted variable is line_break and the change is due to this:

- e = any(AssignRemExpr rem).getLValue()
+ e instanceof DivExpr

The idea behind the original line was that dividing the tainted value could bring it down to a controlled size. The new check also adds a barrier if the tainted value is the denominator. This wasn't intentional, but dividing by an uncontrolled size seems just as likely to lead to a managed size (if not more so since, I'm guessing, allocation sizes are usually integers and then divisions by values <1 that could blow up are ruled out). The change applies to modulo and right shift. Both of these operations can only make the lhs. smaller so the rhs. being tainted does seem like a valid place to stop the taint flow.

I'm a bit puzzled by the alert comparison in the DCA result, where it seems like a bunch of unrelated queries gained a lot of results?

[paldepind]

I think the result we lost with the line m += m / line_break was a false positive, or at least highly dubious, I'm happy we lose it.

Agree.

I've rerun DCA and the new report seems uneventful. The failed tasks and the added query results are gone and only the already mentioned lost query result remains.