go/gosec
1
0
Fork 0
mirror of https://github.com/securego/gosec.git synced 2025-07-03 00:27:05 +02:00
Code Issues Releases Activity

Refactor to support duplicate imports with different aliases (#865)

Browse Source 
The existing code assumed imports to be either imported, or imported with an
alias. Badly formatted files may have duplicate imports for a package, using
different aliases.

This patch refactors the code, and;

Introduces a new `GetImportedNames` function, which returns all name(s) and
aliase(s) for a package, which effectively combines `GetAliasedName` and
`GetImportedName`, but adding support for duplicate imports.

The old `GetAliasedName` and `GetImportedName` functions have been rewritten to
use the new function and marked deprecated, but could be removed if there are no
external consumers.

With this patch, the linter is able to detect issues in files such as;

    package main

    import (
        crand "crypto/rand"
        "math/big"
        "math/rand"
        rand2 "math/rand"
        rand3 "math/rand"
    )

    func main() {
        _, _ = crand.Int(crand.Reader, big.NewInt(int64(2))) // good

        _ = rand.Intn(2) // bad
        _ = rand2.Intn(2)  // bad
        _ = rand3.Intn(2)  // bad
    }

Before this patch, only a single issue would be detected:

    gosec --quiet .

    [main.go:14] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
        13:
      > 14: 	_ = rand.Intn(2) // bad
        15: 	_ = rand2.Intn(2)  // bad

With this patch, all issues are identified:

    gosec --quiet .

    [main.go:16] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
        15: 	_ = rand2.Intn(2)  // bad
      > 16: 	_ = rand3.Intn(2)  // bad
        17: }

    [main.go:15] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
        14: 	_ = rand.Intn(2) // bad
      > 15: 	_ = rand2.Intn(2)  // bad
        16: 	_ = rand3.Intn(2)  // bad

    [main.go:14] - G404 (CWE-338): Use of weak random number generator (math/rand instead of crypto/rand) (Confidence: MEDIUM, Severity: HIGH)
        13:
      > 14: 	_ = rand.Intn(2) // bad
        15: 	_ = rand2.Intn(2)  // bad

While working on this change, I noticed that ImportTracker.TrackFile() was not able
to find import aliases;  Analyser.Check() called both ImportTracker.TrackFile() and
ast.Walk(), which (with the updated ImportTracker) resulted in importes to be in-
correctly included multiple times (once with the correct alias, once with the default).

I updated ImportTracker.TrackFile() to fix this, but with the updated ImportTracker,
Analyser.Check() no longer has to call ImportTracker.TrackFile() separately, as ast.Walk()
already handles the file, and will find all imports.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn
2022-10-17 10:59:18 +02:00
committed by GitHub
parent a2719d3248
commit 0ae0174c25
5 changed files with 71 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
helpers.go
View File

@ -37,12 +37,9 @@ import (
//
// node, matched := MatchCallByPackage(n, ctx, "math/rand", "Read")
func MatchCallByPackage(n ast.Node, c *Context, pkg string, names ...string) (*ast.CallExpr, bool) {
importedName, found := GetAliasedName(pkg, c)
importedNames, found := GetImportedNames(pkg, c)
if !found {
importedName, found = GetImportedName(pkg, c)
if !found {
return nil, false
}
return nil, false
}
if callExpr, ok := n.(*ast.CallExpr); ok {
@ -50,7 +47,10 @@ func MatchCallByPackage(n ast.Node, c *Context, pkg string, names ...string) (*a
if err != nil {
return nil, false
}
if packageName == importedName {
for _, in := range importedNames {
if packageName != in {
continue
}
for _, name := range names {
if callName == name {
return callExpr, true
@ -247,48 +247,23 @@ func GetBinaryExprOperands(be *ast.BinaryExpr) []ast.Node {
return result
}
// 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) {
importName, imported := ctx.Imports.Imported[path]
if !imported {
return "", false
}
if _, initonly := ctx.Imports.InitOnly[path]; initonly {
return "", false
}
return importName, true
}
// GetAliasedName returns the aliased name used for the package within the
// code. It will ignore initialization only imports.
func GetAliasedName(path string, ctx *Context) (string, bool) {
importName, imported := ctx.Imports.Aliased[path]
if !imported {
return "", false
}
if _, initonly := ctx.Imports.InitOnly[path]; initonly {
return "", false
}
return importName, true
// GetImportedNames returns the name(s)/alias(es) used for the package within
// the code. It ignores initialization-only imports.
func GetImportedNames(path string, ctx *Context) (names []string, found bool) {
importNames, imported := ctx.Imports.Imported[path]
return importNames, imported
}
// GetImportPath resolves the full import path of an identifier based on
// the imports in the current context(including aliases).
func GetImportPath(name string, ctx *Context) (string, bool) {
for path := range ctx.Imports.Imported {
if imported, ok := GetImportedName(path, ctx); ok && imported == name {
return path, true
}
}
for path := range ctx.Imports.Aliased {
if imported, ok := GetAliasedName(path, ctx); ok && imported == name {
return path, true
if imported, ok := GetImportedNames(path, ctx); ok {
for _, n := range imported {
if n == name {
return path, true
}
}
}
}