diff --git a/rules/ssrf.go b/rules/ssrf.go index 34aa5d4..b1409a5 100644 --- a/rules/ssrf.go +++ b/rules/ssrf.go @@ -24,8 +24,15 @@ func (r *ssrf) ResolveVar(n *ast.CallExpr, c *gosec.Context) bool { arg := n.Args[0] if ident, ok := arg.(*ast.Ident); ok { obj := c.Info.ObjectOf(ident) - if _, ok := obj.(*types.Var); ok && !gosec.TryResolve(ident, c) { - return true + if _, ok := obj.(*types.Var); ok { + scope := c.Pkg.Scope() + if scope != nil && scope.Lookup(ident.Name) != nil { + // a URL defined in a variable at package scope can be changed at any time + return true + } + if !gosec.TryResolve(ident, c) { + return true + } } } } diff --git a/testutils/source.go b/testutils/source.go index 8d7360c..8cb16a3 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -343,8 +343,8 @@ func main() { } fmt.Printf("%s", body) }`}, 1, gosec.NewConfig()}, {[]string{` -// A variable value can easily be changed no matter -// if it's a global or a local one +// Variable defined a package level can be changed at any time +// regardless of the initial value package main import ( @@ -356,7 +356,6 @@ import ( var url string = "https://www.google.com" func main() { - resp, err := http.Get(url) if err != nil { panic(err) @@ -389,7 +388,7 @@ func main() { } fmt.Printf("%s", body) }`}, 1, gosec.NewConfig()}, {[]string{` -// Constant variables or harcoded strings are secure +// Constant variables or hard-coded strings are secure package main import ( @@ -401,9 +400,96 @@ func main() { resp, err := http.Get(url) if err != nil { fmt.Println(err) - } - fmt.Println(resp.Status) -}`}, 0, gosec.NewConfig()}} + } + fmt.Println(resp.Status) +}`}, 0, gosec.NewConfig()}, {[]string{` +// A variable at function scope which is initialized to +// a constant string is secure (e.g. cannot be changed concurrently) +package main + +import ( + "fmt" + "net/http" +) +func main() { + var url string = "http://127.0.0.1" + resp, err := http.Get(url) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +}`}, 0, gosec.NewConfig()}, {[]string{` +// A variable at function scope which is initialized to +// a constant string is secure (e.g. cannot be changed concurrently) +package main + +import ( + "fmt" + "net/http" +) +func main() { + url := "http://127.0.0.1" + resp, err := http.Get(url) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +}`}, 0, gosec.NewConfig()}, {[]string{` +// A variable at function scope which is initialized to +// a constant string is secure (e.g. cannot be changed concurrently) +package main + +import ( + "fmt" + "net/http" +) +func main() { + url1 := "test" + var url2 string = "http://127.0.0.1" + url2 = url1 + resp, err := http.Get(url2) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +}`}, 0, gosec.NewConfig()}, {[]string{` +// An exported variable declared a packaged scope is not secure +// because it can changed at any time +package main + +import ( + "fmt" + "net/http" +) + +var Url string + +func main() { + resp, err := http.Get(Url) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +}`}, 1, gosec.NewConfig()}, {[]string{` +// An url provided as a function argument is not secure +package main + +import ( + "fmt" + "net/http" +) +func get(url string) { + resp, err := http.Get(url) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +} +func main() { + url := "http://127.0.0.1" + get(url) +}`}, 1, gosec.NewConfig()}} + // SampleCodeG108 - pprof endpoint automatically exposed SampleCodeG108 = []CodeSample{{[]string{` package main