Fix the rule G304 to handle the case when the input is cleaned as a variable assignment

Signed-off-by: Cosmin Cojocar <ccojocar@cloudbees.com>

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")

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{`