IncorrectIntegerConversionQuery precision is overly ambitious and its help should warn about false positives

[jsoref]

trafficstars

https://github.com/github/codeql/blob/590e93d8edec4d7216935ed4425a7ab77b3b2f34/go/ql/src/Security/CWE-681/IncorrectIntegerConversionQuery.ql#L13

We've seen people trying to "fix" reports based on this tooling. I spent some time tracing the flow of one such incident: https://github.com/argoproj/argo-cd/pull/18436#issuecomment-2359634170

For my reference, I used https://github.com/check-spelling-sandbox/argo-cd/security/code-scanning/3

@jketema wrote:

The @precision very-high on the query suggests that there should near 0 false positives on this query, maybe the precision should be lowered?

@owen-mc wrote:

We do try to detect bounds checks, but we can only do it when the value you are checking against is a constant, and in this case you pass a constant into the function and then use the corresponding parameter as the bound, which our analysis isn't able to detect. ... It is a fair comment that the precision of that query should be lowered to high. At the same time, I can try to explain in the qhelp file which ways of fixing this problem we try to detect, and when they don't work.

The current help doesn't warn about bounds checks that could be sufficient but which aren't understood by the checker: https://github.com/github/codeql/blob/590e93d8edec4d7216935ed4425a7ab77b3b2f34/go/ql/src/Security/CWE-681/IncorrectIntegerConversionQuery.qhelp#L18-L28

---

• [x] https://github.com/github/codeql/pull/17571
• [ ] https://github.com/github/codeql/pull/17717