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>
* gha: remove go1.17, temporarily force 1.18.7, 1.19.2
The security scanner is flagging the code to have a vulnerability, but it's
detecting that we're running go1.18.6, not "latest" (go1.18.7 at time of writing).
Temporarily pinning to go1.18.7 to force installing the latest version:
Vulnerability #1: GO-2022-1039
Programs which compile regular expressions from untrusted
sources may be vulnerable to memory exhaustion or denial of
service. The parsed regexp representation is linear in the size
of the input, but in some cases the constant factor can be as
high as 40,000, making relatively small regexps consume much
larger amounts of memory. After fix, each regexp being parsed is
limited to a 256 MB memory footprint. Regular expressions whose
representation would use more space than that are rejected.
Normal use of regular expressions is unaffected.
Call stacks in your code:
Error: helpers.go:463:26: github.com/securego/gosec/v2.ExcludedDirsRegExp calls regexp.MustCompile, which eventually calls regexp/syntax.Parse
Found in: regexp/syntax@go1.18.6
Fixed in: regexp/syntax@go1.19.2
More info: https://pkg.go.dev/vuln/GO-2022-1039
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
* go.mod: github.com/onsi/ginkgo/v2 v2.3.1
CI was failing because of a mismatch:
/home/runner/go/bin/ginkgo -v --fail-fast
Ginkgo detected a version mismatch between the Ginkgo CLI and the version of Ginkgo imported by your packages:
Ginkgo CLI Version:
2.3.1
Mismatched package versions found:
2.2.0 used by gosec
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
* go.mod: golang.org/x/text v0.3.8
to address GO-2022-1059
The vulnerabilities below are in packages that you import, but your code
doesn't appear to call any vulnerable functions. You may not need to take any
action. See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck
for details.
Vulnerability #1: GO-2022-1059
An attacker may cause a denial of service by crafting an Accept-Language
header which ParseAcceptLanguage will take significant time to parse.
Found in: golang.org/x/text/language@v0.3.7
Fixed in: golang.org/x/text/language@v0.3.8
More info: https://pkg.go.dev/vuln/GO-2022-1059
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
