rules/sdk: is it pedantic to reject reflect, runtime, math/rand, crypto/rand?

[odeke-em]

The purpose of this issue is to ask if we are being pedantic about some of these imports:

If we look at https://github.com/informalsystems/gosec/blob/a284576b08668f2835734b359b27faea587a98b1/rules/sdk/blocklist.go#L71-L78

we can see a bunch of critical packages rejected, but I see lots of valid uses for a bunch of them like: a) reflect: used in the SDK when interfaces are passed into say (*BaseApp).MountStores to return a correct error message indicating that an interface wasn't accepted -- this one is okay to flag as wrong per https://github.com/cosmos/cosmos-sdk/pull/10392 but there are so many instances where using reflect is warranted like https://github.com/cosmos/cosmos-sdk/search?q=reflect b) runtime is used in cosmosvisor to retrieve the operating system and architecture as well as for finding stack traces per https://github.com/cosmos/cosmos-sdk/search?q=runtime c) math/rand: we use rand seeded to generate random numbers d) crypto/rand: we use rand to generate private keys in crypto/keys/itnernal/ecdsa in GenPrivKey

[ebuchman]

so i think anywhere we're using this stuff we should flag it. in general there will surely be reflect and possibly runtime in the core, but I think these linters are more geared for the /x modules (tho we should probably use them on the core too).

Any use of rand is suspect and should really only exist in tests since if we generate a random number in the blockchain app we'll have non-determinism.

[ebuchman]

These tools are going to generate lots of false positives and there's really no easy way around that so maybe there's some kind of process/tooling we can add to set certain instances to ignore or something ?

I guess we could add annotations in the code but that's a bit of a pain

[tomtau]

if it integrates with Code Scanning on GH: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning I guess this can be set with properties.precision: low and properties.problem.severity: warning, so at least on the Security tabs, it's ordered not to show up at the top and one can dismiss it as a false positive: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/managing-code-scanning-alerts-for-your-repository#dismissing-or-deleting-alerts