C++: Fix more FPs in cpp/overflow-buffer

[MathiasVP]

This PR is a follow-up to #18615. This PR fixes an annoying case that happens when people write code such as:

struct S {
  struct {
    uint32_t x1;
  };
  struct {
    uint32_t x2;
  };

  void test() {
    memset(&x1, 0, sizeof(S) - offsetof(S, x1));
  }
};

The logic in #18615 wasn't enough to capture this since that PR computed the size of the buffer x1 as x1.getDeclaringType().getSize() - x1.getByteOffset() and since x1.getDeclaringType() is the anonymous struct (of size 4 bytes) and the offset is 0 we conclude that the buffer is size 4, but in reality it should be of size 8.

This PR makes use of a helper predicate initially used only in https://github.com/github/codeql/blob/main/cpp/ql/src/Security/CWE/CWE-843/TypeConfusion.ql by moving the implementation to a public member predicate on Field and generalizing the above "buffer size" calculation so that it works on nested unions/structs.

This fixes a large class of FPs internally at Microsoft 🎉

I've not done a change note since this change is covered by the change note in #18615

[Copilot]

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

[jketema]

Some initial comments.

[jketema]

So "size" here includes everything between the start of s2.s.y and the end of the outermost struct? Is that correct? If so why is that a sensible notion of "size"?

[MathiasVP]

Correct. This is sensible when you consider how S2 is laid out in memory:

x	y	z

So we compute the buffer starting at y by computing the size of the outermost struct (i.e., S2 in this case) which includes x, y, and z. We subtract how many bytes we've already "passed" in order to get to y. So the total size is the size of y + the size of z.

[jketema]

Right, so you're basically interested in not writing beyond the end of the struct?

[MathiasVP]

Yep!

[jketema]

I think we do need a change note for the predicate that's being added here.

[MathiasVP]

Oh, that's a good point. I'll add that

[MathiasVP]

Fixed in 02cf458

[jketema]

LGTM