Linter: gosec, Rule: G404 - Insecure random number source (rand). Should we enable it?

[zak-pawel]

Use Case

This issue starts discussion about enabling:

• linter: gosec - Inspects source code for security problems by scanning the Go AST.
• rule: G404 - Insecure random number source (rand)

Rule is mapped to CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG).

Expected behavior

Decision if rule should be enabled or not.

Actual behavior

For this rule following findings were found in current code:

internal/internal.go:128:13                                    gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
internal/internal.go:146:13                                    gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:449:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:450:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:451:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:452:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:453:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:454:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:485:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:486:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:487:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:488:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:489:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:490:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:526:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:527:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:528:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:529:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:530:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:531:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:562:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:563:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:564:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:565:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:566:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:567:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:603:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:604:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:605:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:606:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:607:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:608:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:639:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:640:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:641:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:642:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:643:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/aggregators/quantile/quantile_test.go:644:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/common/shim/processor_test.go:27:25                    gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/example/example.go:106:13                       gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/mesos/mesos_test.go:275:22                      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/mesos/mesos_test.go:280:21                      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/mock/mock.go:69:11                              gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/powerdns_recursor/protocol_v1.go:22:18          gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/powerdns_recursor/protocol_v2.go:20:18          gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/sql/sql_test.go:25:35                           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/statsd/running_stats.go:84:11                   gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:27:49                 gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:65:49                 gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:118:49                gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:162:49                gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:210:49                gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/suricata/suricata_test.go:236:33                gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/vsphere/endpoint.go:572:13                      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/cloudwatch_logs/cloudwatch_logs_test.go:91:22  gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/event_hubs/event_hubs_test.go:89:7             gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:68:55      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:78:53      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:80:21      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:87:38      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:89:33      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_bench_test.go:89:68      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/postgresql/postgresql_test.go:779:33           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/outputs/sql/sql_test.go:46:35                          gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)

Additional info

For this rule no additional configuration can be provided.

[powersj]

+1 - Looks like primarily tests

[srebhan]

Hmmm we do not necessarily need cryptographic secure RNGs e.g. for quantile sample. My main concern is that with crypto/rand you do drain the entropy pool of the OS while math/rand just uses a pseudo-RNG with a freely chosen seed.

[zak-pawel]

@srebhan So maybe we can filter out findings in tests:

    - path: _test\.go
      text: "Use of weak random number generator"

And review findings for production code only:

internal/internal.go:128:13                            gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
internal/internal.go:146:13                            gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/example/example.go:106:13               gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/mock/mock.go:69:11                      gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/powerdns_recursor/protocol_v1.go:22:18  gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/powerdns_recursor/protocol_v2.go:20:18  gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/statsd/running_stats.go:84:11           gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)
plugins/inputs/vsphere/endpoint.go:572:13              gosec  G404: Use of weak random number generator (math/rand instead of crypto/rand)

Will you have time to take a look? Or maybe even to open a PR? ;)

[srebhan]

@zak-pawel +1 for ignoring the tests. Regarding your question: No time until mid of May so I don't think you can expect a PR from my side... :-(