False positive: Go x, _ := strconv.ParseUint(,, strconv.IntSize-1); int(x)

[thediveo]

Description of the false positive

If I'm not mistaken and making a terribly stupid mistake here I would expect that Go's strconv.IntSize-1 should cover the positive range of an int, regardless of architecture integer size. The actual value range returned by strconv.ParseUint in an uint64 should be correctly castable to an int without ambiguities.

Currently, CodeQL scanning reports "Incorrect conversion between integer types". Am I getting this one horribly wrong?

Code samples or links to source code

fd, err := strconv.ParseUint(fdInfoEntry.Name(), 10, strconv.IntSize-1)
if err != nil {
    continue
}
taptunFd, err := unix.PidfdGetfd(pidfd, int(fd), 0)

https://github.com/siemens/ghostwire/pull/29/checks?check_run_id=18520023133

Related: #9295 ... but it seems the PR addressing the "old" issue doesn't address this case too.

[thediveo]

The reason to parse as uint is because fd numbers cannot be validly negative, so the parsing should immediately fail in case the underlying process filesystem is broken.

[thediveo]

My (hopefully correct) reasoning here is somewhat analogous to path sanitizing where the CodeQL rules "understand" certain stdlib functions to correctly sanitize their return values. Analogous to this I would expect the CodeQL scan to "understand" that the correct usage of strconv.ParseUint already constitutes the required upper bound checks, so no further duplicate bounds check is required.

[owen-mc]

Thanks for reporting this. It is indeed a false positive. It's because I haven't put in special handling for strconv.IntSize to tell the query that it relates to the architecture integer size. As it's run on a 64-bit architecture, it just sees it as the constant 64, and it reasons that on a 32-bit platform there would be a problem.

I have put in some special casing for comparisons with math.MaxInt and math.MaxUint because of similar FP reports in the past, so I should be able to do something similar. It's a little more complicated because you're subtracting 1 from it rather than just using it on its own. I'll try to find a way of doing it that isn't too complicated.