From 19bda8d15fddd732ff8ceec2b7104b79465e5583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Mon, 3 Jan 2022 22:58:25 +0200 Subject: [PATCH] Find more tempdirs * Find G303 in string concatenations, with os.TempDir, and in path.Join args * Find G303 with /usr/tmp, too /usr/tmp is commonly found e.g. on Solaris. --- rules/tempfiles.go | 42 +++++++++++++++++++++++++++++++++++------- testutils/source.go | 23 ++++++++++++++++++++++- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/rules/tempfiles.go b/rules/tempfiles.go index a2aed07..1eb2d73 100644 --- a/rules/tempfiles.go +++ b/rules/tempfiles.go @@ -23,19 +23,41 @@ import ( type badTempFile struct { gosec.MetaData - calls gosec.CallList - args *regexp.Regexp + calls gosec.CallList + args *regexp.Regexp + argCalls gosec.CallList + nestedCalls gosec.CallList } func (t *badTempFile) ID() string { return t.MetaData.ID } +func (t *badTempFile) findTempDirArgs(n ast.Node, c *gosec.Context, suspect ast.Node) *gosec.Issue { + if s, e := gosec.GetString(suspect); e == nil { + if t.args.MatchString(s) { + return gosec.NewIssue(c, n, t.ID(), t.What, t.Severity, t.Confidence) + } + return nil + } + if ce := t.argCalls.ContainsPkgCallExpr(suspect, c, false); ce != nil { + return gosec.NewIssue(c, n, t.ID(), t.What, t.Severity, t.Confidence) + } + if be, ok := suspect.(*ast.BinaryExpr); ok { + if ops := gosec.GetBinaryExprOperands(be); len(ops) != 0 { + return t.findTempDirArgs(n, c, ops[0]) + } + return nil + } + if ce := t.nestedCalls.ContainsPkgCallExpr(suspect, c, false); ce != nil { + return t.findTempDirArgs(n, c, ce.Args[0]) + } + return nil +} + func (t *badTempFile) Match(n ast.Node, c *gosec.Context) (gi *gosec.Issue, err error) { if node := t.calls.ContainsPkgCallExpr(n, c, false); node != nil { - if arg, e := gosec.GetString(node.Args[0]); t.args.MatchString(arg) && e == nil { - return gosec.NewIssue(c, n, t.ID(), t.What, t.Severity, t.Confidence), nil - } + return t.findTempDirArgs(n, c, node.Args[0]), nil } return nil, nil } @@ -45,9 +67,15 @@ func NewBadTempFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { calls := gosec.NewCallList() calls.Add("io/ioutil", "WriteFile") calls.AddAll("os", "Create", "WriteFile") + argCalls := gosec.NewCallList() + argCalls.Add("os", "TempDir") + nestedCalls := gosec.NewCallList() + nestedCalls.Add("path", "Join") return &badTempFile{ - calls: calls, - args: regexp.MustCompile(`^/tmp/.*$|^/var/tmp/.*$`), + calls: calls, + args: regexp.MustCompile(`^(/(usr|var))?/tmp(/.*)?$`), + argCalls: argCalls, + nestedCalls: nestedCalls, MetaData: gosec.MetaData{ ID: id, Severity: gosec.Medium, diff --git a/testutils/source.go b/testutils/source.go index 2d2c6fd..4bdbf72 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -1758,6 +1758,7 @@ import ( "fmt" "io/ioutil" "os" + "path" ) func main() { @@ -1775,7 +1776,27 @@ func main() { if err != nil { fmt.Println("Error while writing!") } -}`}, 3, gosec.NewConfig()}} + err = os.WriteFile("/usr/tmp/demo2", []byte("This is some data"), 0644) + if err != nil { + fmt.Println("Error while writing!") + } + err = os.WriteFile("/tmp/" + "demo2", []byte("This is some data"), 0644) + if err != nil { + fmt.Println("Error while writing!") + } + err = os.WriteFile(os.TempDir() + "/demo2", []byte("This is some data"), 0644) + if err != nil { + fmt.Println("Error while writing!") + } + err = os.WriteFile(path.Join("/var/tmp", "demo2"), []byte("This is some data"), 0644) + if err != nil { + fmt.Println("Error while writing!") + } + err = os.WriteFile(path.Join(os.TempDir(), "demo2"), []byte("This is some data"), 0644) + if err != nil { + fmt.Println("Error while writing!") + } +}`}, 8, gosec.NewConfig()}} // SampleCodeG304 - potential file inclusion vulnerability SampleCodeG304 = []CodeSample{{[]string{`