C++: Fix indirect taint

[MathiasVP]

The motivation for this PR was that we failed to track this flow when the source is whatever indirect_source() points to:

void test_indirect_taint() {
  std::map<int, int*> m;
  int* p = indirect_source();
  m[1] = p;
  int* q = m[1];
  sink(q); // $ MISSING: ast ir
}

The fix was in our interpretation of models (in ModelUtil.qll) so that the model for operator[] correctly carried indirect flow from *p to m (this causes pointer/pointee conflation but since these are taint models that's fine).

Finally, fixing the interpretation of models revealed a missing flow step for smart pointers. Specifically, we didn't model that operator-> and get performed a load.

[MathiasVP]

@jketema I've assigned you as a reviewer since it fixes a problem you've observed. Let me know if you need any assistance on the reviewing side 🙇!

(the force-push was just me messing around - it's the same code that was tested on DCA)

[jketema]

Why are the subtractions needed here?

[MathiasVP]

Yeah, I'm not quite happy with that. The indirection index of IndirectArgumentOutNode is defined to be [1 .. n] (up to some number of indirections), but in the caller of this predicate (callOutput/3) the not exists(getIndirectReturnOutNode(call, indirectionIndex + d)) predicate expects the indirection index to be defined [0 .. n - 1] (because that's the indirection index range of IndirectReturnOutNode since it can be both a "direct return" with indirection index being 0, or an indirect return with indirection index being >= 1).

So while it's very difficult to reason about this indirection index, it's very easy to see when they're wrong since any other offsets added here breaks some of our tests 😂. I'd like to clean up this so that we don't need these subtractions at some point, but I'd rather not add too much more onto this PR.

[jketema]

Why is this no longer needed?

[MathiasVP]

This is now handled by the result = callOutputWithIndirectionIndex(call, output, indirectionIndex + d) line because callOutputWithIndirectionIndex has a case for IndirectArgumentOutNode. So when we have:

n = callOutputWithIndirectionIndex(call, output, indirectionIndex) and
result = callOutputWithIndirectionIndex(call, output, indirectionIndex + d)

in callOutput, we get an equivalent condition.

Does that make sense?

[jketema]

Partially. My main problem is, that it's rather hard for me to map the old code on the new code, because it uses different predicates.

[MathiasVP]

Yeah, it's not an easy commit to review as it's not a clear line-for-line mapping 😢. The main point of the commit is to get an indirection index out of callOutput/2 (which has been renamed to callOutputWithIndirectionIndex) so that we can compute indirectionIndex + d on line 92 to "increment" the number of indirections of the resulting node by d.