Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 2 tasks
jsoref opened this issue Sep 25, 2024 · 1 comment
Assignees

Comments

@jsoref
Copy link
Contributor

jsoref commented Sep 25, 2024

We've seen people trying to "fix" reports based on this tooling. I spent some time tracing the flow of one such incident:
argoproj/argo-cd#18436 (comment)

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:

If you need to parse integer values with specific bit sizes, avoid <code>strconv.Atoi</code>, and instead
use <code>strconv.ParseInt</code> or <code>strconv.ParseUint</code>, which also allow specifying the
bit size.
</p>
<p>
When using those functions, be careful to not convert the result to another type with a smaller bit size than
the bit size you specified when parsing the number.
</p>
<p>
If this is not possible, then add upper (and lower) bound checks specific to each type and
bit size (you can find the minimum and maximum value for each type in the <code>math</code> package).


@owen-mc owen-mc self-assigned this Sep 25, 2024
@owen-mc
Copy link
Contributor

owen-mc commented Oct 9, 2024

I've updated the qhelp in #17717.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants