diff --git a/rules/readfile.go b/rules/readfile.go index 459b4ad..3d40ddb 100644 --- a/rules/readfile.go +++ b/rules/readfile.go @@ -25,6 +25,7 @@ type readfile struct { gosec.MetaData gosec.CallList pathJoin gosec.CallList + clean gosec.CallList } // ID returns the identifier for this rule @@ -56,6 +57,21 @@ func (r *readfile) isJoinFunc(n ast.Node, c *gosec.Context) bool { return false } +// isFilepathClean checks if there is a filepath.Clean before assigning to a variable +func (r *readfile) isFilepathClean(n *ast.Ident, c *gosec.Context) bool { + if n.Obj.Kind != ast.Var { + return false + } + if node, ok := n.Obj.Decl.(*ast.AssignStmt); ok { + if call, ok := node.Rhs[0].(*ast.CallExpr); ok { + if clean := r.clean.ContainsPkgCallExpr(call, c, false); clean != nil { + return true + } + } + } + return false +} + // Match inspects AST nodes to determine if the match the methods `os.Open` or `ioutil.ReadFile` func (r *readfile) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { if node := r.ContainsPkgCallExpr(n, c, false); node != nil { @@ -77,7 +93,9 @@ func (r *readfile) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { if ident, ok := arg.(*ast.Ident); ok { obj := c.Info.ObjectOf(ident) - if _, ok := obj.(*types.Var); ok && !gosec.TryResolve(ident, c) { + if _, ok := obj.(*types.Var); ok && + !gosec.TryResolve(ident, c) && + !r.isFilepathClean(ident, c) { return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } @@ -90,6 +108,7 @@ func (r *readfile) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { func NewReadFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { rule := &readfile{ pathJoin: gosec.NewCallList(), + clean: gosec.NewCallList(), CallList: gosec.NewCallList(), MetaData: gosec.MetaData{ ID: id, @@ -100,6 +119,7 @@ func NewReadFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { } rule.pathJoin.Add("path/filepath", "Join") rule.pathJoin.Add("path", "Join") + rule.clean.Add("path/filepath", "Clean") rule.Add("io/ioutil", "ReadFile") rule.Add("os", "Open") rule.Add("os", "OpenFile") diff --git a/testutils/source.go b/testutils/source.go index 7b17094..d7a88b2 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -1586,7 +1586,42 @@ func main() { log.Printf("Error: %v\n", err) } log.Print(body) -}`}, 1, gosec.NewConfig()}} +}`}, 1, gosec.NewConfig()}, {[]string{` +package main + +import ( + "os" + "path/filepath" +) + +func main() { + repoFile := "path_of_file" + cleanRepoFile := filepath.Clean(repoFile) + byContext, err := os.OpenFile(cleanRepoFile, os.O_RDONLY, 0600) + if err != nil { + panic(err) + } +} +`}, 0, gosec.NewConfig()}, {[]string{` +package main + +import ( + "os" + "path/filepath" +) + +func openFile(filePath string) { + byContext, err := os.OpenFile(filepath.Clean(filePath), os.O_RDONLY, 0600) + if err != nil { + panic(err) + } +} + +func main() { + repoFile := "path_of_file" + openFile(repoFile) +} +`}, 0, gosec.NewConfig()}} // SampleCodeG305 - File path traversal when extracting zip/tar archives SampleCodeG305 = []CodeSample{{[]string{`