mirror of
https://github.com/securego/gosec.git
synced 2024-12-26 04:25:52 +00:00
Fix false positives for G404 with aliased packages
It appears that `GetImportedName` returns _both_ aliased and non-aliased imports. As a result, a file having both crypto/rand and math/rand (but aliased) would trigger false positives on G404. Given the following file; ```go package main import ( "crypto/rand" "math/big" rnd "math/rand" ) func main() { _, _ = rand.Int(rand.Reader, big.NewInt(int64(2))) _ = rnd.Intn(2) } ``` And patching for debugging; ```patch diff --git a/helpers.go b/helpers.go index 437d032..80f4233 100644 --- a/helpers.go +++ b/helpers.go @@ -250,6 +250,8 @@ func GetBinaryExprOperands(be *ast.BinaryExpr) []ast.Node { // GetImportedName returns the name used for the package within the // code. It will ignore initialization only imports. func GetImportedName(path string, ctx *Context) (string, bool) { + fmt.Printf("%+v", ctx.Imports.Imported) + os.Exit(1) importName, imported := ctx.Imports.Imported[path] if !imported { return "", false ``` Would show that `math/rand` was included in the list, using it's non-aliased name (`:rand`). gosec -quiet . map[crypto/rand:rand math/big:big math/rand:rand] This patch works around this problem by reversing the order in which imports are resolved in `MatchCallByPackage()`. Aliased packages are tried first, after which non-aliased imports are tried. Given the example application mentioned above: Before this patch: ```bash gosec -quiet . Results: [/Users/sebastiaan/Projects/test/gosec-issue/main.go:10] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH) 9: func main() { > 10: _, _ = rand.Int(rand.Reader, big.NewInt(int64(2))) 11: _ = rnd.Intn(2) ``` With this patch applied: ```bash gosec --quiet . Results: [/Users/sebastiaan/Projects/test/gosec-issue/main.go:11] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH) 10: _, _ = rand.Int(rand.Reader, big.NewInt(int64(2))) > 11: _ = rnd.Intn(2) 12: } ``` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
parent
aaaf80c9a7
commit
dfde579243
2 changed files with 18 additions and 2 deletions
|
@ -37,9 +37,9 @@ import (
|
||||||
//
|
//
|
||||||
// node, matched := MatchCallByPackage(n, ctx, "math/rand", "Read")
|
// node, matched := MatchCallByPackage(n, ctx, "math/rand", "Read")
|
||||||
func MatchCallByPackage(n ast.Node, c *Context, pkg string, names ...string) (*ast.CallExpr, bool) {
|
func MatchCallByPackage(n ast.Node, c *Context, pkg string, names ...string) (*ast.CallExpr, bool) {
|
||||||
importedName, found := GetImportedName(pkg, c)
|
importedName, found := GetAliasedName(pkg, c)
|
||||||
if !found {
|
if !found {
|
||||||
importedName, found = GetAliasedName(pkg, c)
|
importedName, found = GetImportedName(pkg, c)
|
||||||
if !found {
|
if !found {
|
||||||
return nil, false
|
return nil, false
|
||||||
}
|
}
|
||||||
|
|
|
@ -3180,6 +3180,22 @@ func main() {
|
||||||
bad := rand.Intn(10)
|
bad := rand.Intn(10)
|
||||||
println(bad)
|
println(bad)
|
||||||
}`}, 1, gosec.NewConfig()},
|
}`}, 1, gosec.NewConfig()},
|
||||||
|
{[]string{`
|
||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"crypto/rand"
|
||||||
|
"math/big"
|
||||||
|
rnd "math/rand"
|
||||||
|
)
|
||||||
|
|
||||||
|
func main() {
|
||||||
|
good, _ := rand.Int(rand.Reader, big.NewInt(int64(2)))
|
||||||
|
println(good)
|
||||||
|
bad := rnd.Intn(2)
|
||||||
|
println(bad)
|
||||||
|
}
|
||||||
|
`}, 1, gosec.NewConfig()},
|
||||||
}
|
}
|
||||||
|
|
||||||
// SampleCodeG501 - Blocklisted import MD5
|
// SampleCodeG501 - Blocklisted import MD5
|
||||||
|
|
Loading…
Reference in a new issue