From f823a7e92baec448723690f77c2a9d037e7534ce Mon Sep 17 00:00:00 2001 From: Rick Moran Date: Wed, 8 Mar 2023 08:42:45 -0500 Subject: [PATCH] Check nil pointer when variable is declared in a different file --- rules/readfile.go | 6 ++++- testutils/source.go | 65 +++++++++++++++++++++++++++++++++++++-------- tools/tools.go | 1 + 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/rules/readfile.go b/rules/readfile.go index cbd1603..5a5f90b 100644 --- a/rules/readfile.go +++ b/rules/readfile.go @@ -81,7 +81,11 @@ func (r *readfile) isFilepathClean(n *ast.Ident, c *gosec.Context) bool { 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 + // ident.Obj may be nil if the referenced declaration is in another file. It also may be incorrect. + // if it is nil, do not follow it. + if ident.Obj != nil { + r.cleanedVar[ident.Obj.Decl] = n + } } } } diff --git a/testutils/source.go b/testutils/source.go index 85701c3..0dc5098 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -2330,7 +2330,8 @@ func main() { } log.Print(body) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2346,7 +2347,8 @@ func main() { } log.Print(body) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2370,7 +2372,8 @@ func main() { fmt.Fprintf(w, "%s", body) }) log.Fatal(http.ListenAndServe(":3000", nil)) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2394,7 +2397,8 @@ func main() { fmt.Fprintf(w, "%s", body) }) log.Fatal(http.ListenAndServe(":3000", nil)) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2410,7 +2414,8 @@ import ( log.Printf("Error: %v\n", err) } log.Print(body) - }`}, 1, gosec.NewConfig()}, {[]string{` + }`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2434,7 +2439,8 @@ func main() { fmt.Printf("Error: %v\n", err) } fmt.Println(string(contents)) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2453,7 +2459,8 @@ func main() { log.Printf("Error: %v\n", err) } log.Print(body) -}`}, 1, gosec.NewConfig()}, {[]string{` +}`}, 1, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2469,7 +2476,8 @@ func main() { panic(err) } } -`}, 0, gosec.NewConfig()}, {[]string{` +`}, 0, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2488,7 +2496,8 @@ func main() { repoFile := "path_of_file" openFile(repoFile) } -`}, 0, gosec.NewConfig()}, {[]string{` +`}, 0, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2510,7 +2519,8 @@ func main() { dir := "path_of_dir" openFile(dir, repoFile) } -`}, 0, gosec.NewConfig()}, {[]string{` +`}, 0, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2530,7 +2540,8 @@ func main() { } } -`}, 0, gosec.NewConfig()}, {[]string{` +`}, 0, gosec.NewConfig()}, + {[]string{` package main import ( @@ -2561,6 +2572,38 @@ func main() { panic(err) } }`}, 1, gosec.NewConfig()}, + {[]string{` +package main + +import ( + "fmt" + "path/filepath" +) + +type foo struct { +} + +func (f *foo) doSomething(silly string) error { + whoCares, err := filepath.Rel(THEWD, silly) + if err != nil { + return err + } + fmt.Printf("%s", whoCares) + return nil +} + +func main() { + f := &foo{} + + if err := f.doSomething("irrelevant"); err != nil { + panic(err) + } +} +`, ` +package main + +var THEWD string +`}, 0, gosec.NewConfig()}, } // SampleCodeG305 - File path traversal when extracting zip/tar archives diff --git a/tools/tools.go b/tools/tools.go index 14d2436..620785e 100644 --- a/tools/tools.go +++ b/tools/tools.go @@ -1,3 +1,4 @@ +//go:build tools // +build tools package tools