From 419c9292c8bf17683b8e27bd986b2262544ca73d Mon Sep 17 00:00:00 2001 From: cschoenduve-splunk <40579479+cschoenduve-splunk@users.noreply.github.com> Date: Mon, 3 Sep 2018 23:55:03 -0700 Subject: [PATCH] G107 - SSRF (#236) * Initial SSRF Rule * Added Selector evaluation * Added source code tests * Fixed spacing issues * Fixed Spacingv2 * Removed resty test --- README.md | 7 ++-- cmd/tlsconfig/tlsconfig.go | 2 +- rules/rulelist.go | 1 + rules/rules_test.go | 4 +++ rules/ssrf.go | 70 ++++++++++++++++++++++++++++++++++++++ testutils/source.go | 69 +++++++++++++++++++++++++++++++++++++ 6 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 rules/ssrf.go diff --git a/README.md b/README.md index 1ea4650..e47ab9a 100644 --- a/README.md +++ b/README.md @@ -41,12 +41,13 @@ or to specify a set of rules to explicitly exclude using the '-exclude=' flag. - G104: Audit errors not checked - G105: Audit the use of math/big.Int.Exp - G106: Audit the use of ssh.InsecureIgnoreHostKey + - G107: Url provided to HTTP request as taint input - G201: SQL query construction using format string - G202: SQL query construction using string concatenation - G203: Use of unescaped data in HTML templates - G204: Audit use of command execution - G301: Poor file permissions used when creating a directory - - G302: Poor file permisions used with chmod + - G302: Poor file permissions used with chmod - G303: Creating tempfile using a predictable path - G304: File path provided as taint input - G305: File traversal when extracting zip archive @@ -79,7 +80,7 @@ that are not considered build artifacts by the compiler (so test files). As with all automated detection tools there will be cases of false positives. In cases where gosec reports a failure that has been manually verified as being safe it is possible to annotate the code with a '#nosec' comment. The annotation causes gosec to stop processing any further nodes within the -AST so can apply to a whole block or more granularly to a single expression. +AST so can apply to a whole block or more granularly to a single expression. ```go @@ -178,7 +179,7 @@ You can build the docker image as follows: make image ``` -You can run the `gosec` tool in a container against your local Go project. You just have to mount the project in the +You can run the `gosec` tool in a container against your local Go project. You just have to mount the project in the `GOPATH` of the container: ``` diff --git a/cmd/tlsconfig/tlsconfig.go b/cmd/tlsconfig/tlsconfig.go index 94bc387..1e62966 100644 --- a/cmd/tlsconfig/tlsconfig.go +++ b/cmd/tlsconfig/tlsconfig.go @@ -62,7 +62,7 @@ type goTLSConfiguration struct { // getTLSConfFromURL retrieves the json containing the TLS configurations from the specified URL. func getTLSConfFromURL(url string) (*ServerSideTLSJson, error) { - r, err := http.Get(url) + r, err := http.Get(url) // #nosec G107 if err != nil { return nil, err } diff --git a/rules/rulelist.go b/rules/rulelist.go index e9685b9..260cd3b 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -65,6 +65,7 @@ func Generate(filters ...RuleFilter) RuleList { {"G104", "Audit errors not checked", NewNoErrorCheck}, {"G105", "Audit the use of big.Exp function", NewUsingBigExp}, {"G106", "Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, + {"G107", "Url provided to HTTP request as taint input", NewSSRFCheck}, // injection {"G201", "SQL query construction using format string", NewSQLStrFormat}, diff --git a/rules/rules_test.go b/rules/rules_test.go index c41e532..0bd0ce9 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -71,6 +71,10 @@ var _ = Describe("gosec rules", func() { runner("G106", testutils.SampleCodeG106) }) + It("should detect ssrf via http requests with variable url", func() { + runner("G107", testutils.SampleCodeG107) + }) + It("should detect sql injection via format strings", func() { runner("G201", testutils.SampleCodeG201) }) diff --git a/rules/ssrf.go b/rules/ssrf.go new file mode 100644 index 0000000..3185517 --- /dev/null +++ b/rules/ssrf.go @@ -0,0 +1,70 @@ +package rules + +import ( + "go/ast" + "go/types" + + "github.com/securego/gosec" +) + +type ssrf struct { + gosec.MetaData + gosec.CallList +} + +// ID returns the identifier for this rule +func (r *ssrf) ID() string { + return r.MetaData.ID +} + +// ResolveVar tries to resolve the first argument of a call expression +// The first argument is the url +func (r *ssrf) ResolveVar(n *ast.CallExpr, c *gosec.Context) bool { + if len(n.Args) > 0 { + 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 + } + } + } + return false +} + +// Match inspects AST nodes to determine if certain net/http methods are called with variable input +func (r *ssrf) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { + // Call expression is using http package directly + if node := r.ContainsCallExpr(n, c); node != nil { + if r.ResolveVar(node, c) { + return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + // Look at the last selector identity for methods matching net/http's + if node, ok := n.(*ast.CallExpr); ok { + if selExpr, ok := node.Fun.(*ast.SelectorExpr); ok { + // Pull last selector's identity name and compare to net/http methods + if r.Contains("net/http", selExpr.Sel.Name) { + if r.ResolveVar(node, c) { + return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + } + } + return nil, nil +} + +// NewSSRFCheck detects cases where HTTP requests are sent +func NewSSRFCheck(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { + rule := &ssrf{ + CallList: gosec.NewCallList(), + MetaData: gosec.MetaData{ + ID: id, + What: "Potential HTTP request made with variable url", + Severity: gosec.Medium, + Confidence: gosec.Medium, + }, + } + rule.AddAll("net/http", "Do", "Get", "Head", "Post", "PostForm", "RoundTrip") + return rule, []ast.Node{(*ast.CallExpr)(nil)} +} diff --git a/testutils/source.go b/testutils/source.go index 48e83b8..34d5c35 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -192,6 +192,75 @@ import ( func main() { _ = ssh.InsecureIgnoreHostKey() }`, 1}} + + // SampleCodeG107 - SSRF via http requests with variable url + SampleCodeG107 = []CodeSample{{` +package main +import ( + "net/http" + "io/ioutil" + "fmt" + "os" +) +func main() { + url := os.Getenv("tainted_url") + resp, err := http.Get(url) + if err != nil { + panic(err) + } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + panic(err) + } + fmt.Printf("%s", body) +}`, 1}, {` +package main + +import ( + "fmt" + "net/http" +) +const url = "http://127.0.0.1" +func main() { + resp, err := http.Get(url) + if err != nil { + fmt.Println(err) + } + fmt.Println(resp.Status) +}`, 0}, {` +package main + +import ( + "net/http" + "fmt" + "os" + "strconv" +) + +type httpWrapper struct { + DesiredCode string +} + +func (c *httpWrapper) Get(url string) (*http.Response, error) { + return http.Get(url) +} + +func main() { + code := os.Getenv("STATUS_CODE") + var url = os.Getenv("URL") + client := httpWrapper{code} + resp1, err1 := client.Get(url) + if err1 != nil { + fmt.Println(err1) + os.Exit(1) + } + if strconv.Itoa(resp1.StatusCode) == client.DesiredCode { + fmt.Println("True") + } else { + fmt.Println("False") + } +}`, 2}} // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ {`