cpp/missing-check-scanf: False positive

[ryao]

https://github.com/ryao/zfs/security/code-scanning/420

https://github.com/ryao/zfs/blob/3881dd42bbfb7297f08e796c38b35d54e11ac500/lib/libspl/os/linux/gethostid.c#L50

CodeQL says This variable is read, but may not have been written. It should be guarded by a check that the returns at least 1.. However, that is already being done as part of if (fscanf(f, "%lx", &hostid) != 1).

[ryao]

Another instance of this is here:

https://github.com/ryao/zfs/security/code-scanning/424

Another instance that was reported twice:

https://github.com/ryao/zfs/security/code-scanning/425
https://github.com/ryao/zfs/security/code-scanning/426

The following all complain about the same fscanf(), although a false positive on this one is somewhat understandable since the code is designed to rely on a default that remains should the fscanf() call fail:

https://github.com/ryao/zfs/security/code-scanning/421
https://github.com/ryao/zfs/security/code-scanning/422
https://github.com/ryao/zfs/security/code-scanning/423

That one depends entirely on programmer intent, so I am not sure what could be done about that. Maybe we could use (void) before fscanf() to indicate that we do not care about the operation's success when the variable is already initialized. However, I vaguely recall the way the code is written was done intentionally to work around a compiler warning that tripped -Wall -Werror. It was also caught by cpp/empty-if and a past attempt to clean it up tripped a bug in GCC's diagnostics. :/

[redsun82]

Hi @ryao, thanks a lot for this report. I can confirm these findings. It seems like you found two different sources of false positives:

• writes to a scanf-like destination in the failing branch of the call do not seem to sanitize the unsafe undeclared variable
• this scanf analysis seems to not work very well with static variables. In particular it seems to ignore that those are always 0-initialised, which seems to throw off the analysis for alerts like https://github.com/ryao/zfs/security/code-scanning/421.

I've added these false positives to our tests, but at this time I cannot give an estimate as to when we will be able to fix this.