Track back when a file path was sanitized with filepath.Clean (#912)

* Track back when a file path was sanitized with filepath.Clean

* Remove unused argument to fix lint warnings
This commit is contained in:
Cosmin Cojocar 2023-01-09 16:26:20 +01:00 committed by GitHub
parent fd280360cd
commit 5874e63c9e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 15 deletions

View file

@ -24,8 +24,9 @@ import (
type readfile struct {
gosec.MetaData
gosec.CallList
pathJoin gosec.CallList
clean gosec.CallList
pathJoin gosec.CallList
clean gosec.CallList
cleanedVar map[any]ast.Node
}
// ID returns the identifier for this rule
@ -57,24 +58,29 @@ 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
}
}
// isFilepathClean checks if there is a filepath.Clean for given variable
func (r *readfile) isFilepathClean(n *ast.Ident) bool {
if _, ok := r.cleanedVar[n.Obj.Decl]; ok {
return true
}
return false
}
// trackFilepathClean tracks back the declaration of variable from filepath.Clean argument
func (r *readfile) trackFilepathClean(n ast.Node) {
if clean, ok := n.(*ast.CallExpr); ok && len(clean.Args) > 0 {
if ident, ok := clean.Args[0].(*ast.Ident); ok {
r.cleanedVar[ident.Obj.Decl] = n
}
}
}
// 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 {
if node := r.clean.ContainsPkgCallExpr(n, c, false); node != nil {
r.trackFilepathClean(n)
return nil, nil
} else if node := r.ContainsPkgCallExpr(n, c, false); node != nil {
for _, arg := range node.Args {
// handles path joining functions in Arg
// eg. os.Open(filepath.Join("/tmp/", file))
@ -95,7 +101,7 @@ func (r *readfile) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
obj := c.Info.ObjectOf(ident)
if _, ok := obj.(*types.Var); ok &&
!gosec.TryResolve(ident, c) &&
!r.isFilepathClean(ident, c) {
!r.isFilepathClean(ident) {
return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil
}
}
@ -116,6 +122,7 @@ func NewReadFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
Severity: gosec.Medium,
Confidence: gosec.High,
},
cleanedVar: map[any]ast.Node{},
}
rule.pathJoin.Add("path/filepath", "Join")
rule.pathJoin.Add("path", "Join")

View file

@ -2463,6 +2463,28 @@ import (
"path/filepath"
)
func openFile(dir string, filePath string) {
fp := filepath.Join(dir, filePath)
fp = filepath.Clean(fp)
_, err := os.OpenFile(fp, os.O_RDONLY, 0600)
if err != nil {
panic(err)
}
}
func main() {
repoFile := "path_of_file"
dir := "path_of_dir"
openFile(dir, repoFile)
}
`}, 0, gosec.NewConfig()}, {[]string{`
package main
import (
"os"
"path/filepath"
)
func main() {
repoFile := "path_of_file"
relFile, err := filepath.Rel("./", repoFile)