Skip to content

Commit

Permalink
Improve the SSRF rule to report an issue for package scoped variables
Browse files Browse the repository at this point in the history
Made also the rule to not report an issue when encountering function
scoped variable which terminate in a basic literal such as a string.

Signed-off-by: Cosmin Cojocar <cosmin.cojocar@gmx.ch>
  • Loading branch information
ccojocar authored and Cosmin Cojocar committed Oct 8, 2019
1 parent 07770ae commit 50e1fe2
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 9 deletions.
11 changes: 9 additions & 2 deletions rules/ssrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
Expand Down
100 changes: 93 additions & 7 deletions testutils/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -356,7 +356,6 @@ import (
var url string = "https://www.google.com"
func main() {
resp, err := http.Get(url)
if err != nil {
panic(err)
Expand Down Expand Up @@ -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 (
Expand All @@ -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
Expand Down

0 comments on commit 50e1fe2

Please sign in to comment.