diff --git a/_linters/src/github.com/securego/gosec/cmd/tlsconfig/tlsconfig.go b/_linters/src/github.com/securego/gosec/cmd/tlsconfig/tlsconfig.go index 94bc387b..1e629661 100644 --- a/_linters/src/github.com/securego/gosec/cmd/tlsconfig/tlsconfig.go +++ b/_linters/src/github.com/securego/gosec/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/_linters/src/github.com/securego/gosec/helpers.go b/_linters/src/github.com/securego/gosec/helpers.go index bc01a94c..638129b1 100644 --- a/_linters/src/github.com/securego/gosec/helpers.go +++ b/_linters/src/github.com/securego/gosec/helpers.go @@ -281,3 +281,33 @@ func ConcatString(n *ast.BinaryExpr) (string, bool) { } return s, true } + +// FindVarIdentities returns array of all variable identities in a given binary expression +func FindVarIdentities(n *ast.BinaryExpr, c *Context) ([]*ast.Ident, bool) { + identities := []*ast.Ident{} + // sub expressions are found in X object, Y object is always the last term + if rightOperand, ok := n.Y.(*ast.Ident); ok { + obj := c.Info.ObjectOf(rightOperand) + if _, ok := obj.(*types.Var); ok && !TryResolve(rightOperand, c) { + identities = append(identities, rightOperand) + } + } + if leftOperand, ok := n.X.(*ast.BinaryExpr); ok { + if leftIdentities, ok := FindVarIdentities(leftOperand, c); ok { + identities = append(identities, leftIdentities...) + } + } else { + if leftOperand, ok := n.X.(*ast.Ident); ok { + obj := c.Info.ObjectOf(leftOperand) + if _, ok := obj.(*types.Var); ok && !TryResolve(leftOperand, c) { + identities = append(identities, leftOperand) + } + } + } + + if len(identities) > 0 { + return identities, true + } + // if nil or error, return false + return nil, false +} diff --git a/_linters/src/github.com/securego/gosec/rules/readfile.go b/_linters/src/github.com/securego/gosec/rules/readfile.go index 61e1c859..2b388520 100644 --- a/_linters/src/github.com/securego/gosec/rules/readfile.go +++ b/_linters/src/github.com/securego/gosec/rules/readfile.go @@ -24,6 +24,7 @@ import ( type readfile struct { gosec.MetaData gosec.CallList + pathJoin gosec.CallList } // ID returns the identifier for this rule @@ -31,10 +32,49 @@ func (r *readfile) ID() string { return r.MetaData.ID } +// isJoinFunc checks if there is a filepath.Join or other join function +func (r *readfile) isJoinFunc(n ast.Node, c *gosec.Context) bool { + if call := r.pathJoin.ContainsCallExpr(n, c); call != nil { + for _, arg := range call.Args { + // edge case: check if one of the args is a BinaryExpr + if binExp, ok := arg.(*ast.BinaryExpr); ok { + // iterate and resolve all found identites from the BinaryExpr + if _, ok := gosec.FindVarIdentities(binExp, c); ok { + return true + } + } + + // try and resolve identity + 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 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.ContainsCallExpr(n, c); node != nil { for _, arg := range node.Args { + // handles path joining functions in Arg + // eg. os.Open(filepath.Join("/tmp/", file)) + if callExpr, ok := arg.(*ast.CallExpr); ok { + if r.isJoinFunc(callExpr, c) { + return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + // handles binary string concatenation eg. ioutil.Readfile("/tmp/" + file + "/blob") + if binExp, ok := arg.(*ast.BinaryExpr); ok { + // resolve all found identites from the BinaryExpr + if _, ok := gosec.FindVarIdentities(binExp, c); ok { + return gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + if ident, ok := arg.(*ast.Ident); ok { obj := c.Info.ObjectOf(ident) if _, ok := obj.(*types.Var); ok && !gosec.TryResolve(ident, c) { @@ -49,6 +89,7 @@ func (r *readfile) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { // NewReadFile detects cases where we read files func NewReadFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { rule := &readfile{ + pathJoin: gosec.NewCallList(), CallList: gosec.NewCallList(), MetaData: gosec.MetaData{ ID: id, @@ -57,6 +98,8 @@ func NewReadFile(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { Confidence: gosec.High, }, } + rule.pathJoin.Add("path/filepath", "Join") + rule.pathJoin.Add("path", "Join") rule.Add("io/ioutil", "ReadFile") rule.Add("os", "Open") return rule, []ast.Node{(*ast.CallExpr)(nil)} diff --git a/_linters/src/github.com/securego/gosec/rules/rulelist.go b/_linters/src/github.com/securego/gosec/rules/rulelist.go index e9685b9e..260cd3bf 100644 --- a/_linters/src/github.com/securego/gosec/rules/rulelist.go +++ b/_linters/src/github.com/securego/gosec/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/_linters/src/github.com/securego/gosec/rules/sql.go b/_linters/src/github.com/securego/gosec/rules/sql.go index fe7732da..38f85986 100644 --- a/_linters/src/github.com/securego/gosec/rules/sql.go +++ b/_linters/src/github.com/securego/gosec/rules/sql.go @@ -98,16 +98,35 @@ func NewSQLStrConcat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { type sqlStrFormat struct { sqlStatement - calls gosec.CallList + calls gosec.CallList + noIssue gosec.CallList } // Looks for "fmt.Sprintf("SELECT * FROM foo where '%s', userInput)" func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { + // argIndex changes the function argument which gets matched to the regex + argIndex := 0 + // TODO(gm) improve confidence if database/sql is being used if node := s.calls.ContainsCallExpr(n, c); node != nil { + // if the function is fmt.Fprintf, search for SQL statement in Args[1] instead + if sel, ok := node.Fun.(*ast.SelectorExpr); ok { + if sel.Sel.Name == "Fprintf" { + // if os.Stderr or os.Stdout is in Arg[0], mark as no issue + if arg, ok := node.Args[0].(*ast.SelectorExpr); ok { + if ident, ok := arg.X.(*ast.Ident); ok { + if s.noIssue.Contains(ident.Name, arg.Sel.Name) { + return nil, nil + } + } + } + // the function is Fprintf so set argIndex = 1 + argIndex = 1 + } + } // concats callexpr arg strings together if needed before regex evaluation - if argExpr, ok := node.Args[0].(*ast.BinaryExpr); ok { + if argExpr, ok := node.Args[argIndex].(*ast.BinaryExpr); ok { if fullStr, ok := gosec.ConcatString(argExpr); ok { if s.MatchPatterns(fullStr) { return gosec.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), @@ -116,7 +135,7 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) } } - if arg, e := gosec.GetString(node.Args[0]); s.MatchPatterns(arg) && e == nil { + if arg, e := gosec.GetString(node.Args[argIndex]); s.MatchPatterns(arg) && e == nil { return gosec.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), nil } } @@ -126,7 +145,8 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) // NewSQLStrFormat looks for cases where we're building SQL query strings using format strings func NewSQLStrFormat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { rule := &sqlStrFormat{ - calls: gosec.NewCallList(), + calls: gosec.NewCallList(), + noIssue: gosec.NewCallList(), sqlStatement: sqlStatement{ patterns: []*regexp.Regexp{ regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), @@ -140,6 +160,7 @@ func NewSQLStrFormat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { }, }, } - rule.calls.AddAll("fmt", "Sprint", "Sprintf", "Sprintln") + rule.calls.AddAll("fmt", "Sprint", "Sprintf", "Sprintln", "Fprintf") + rule.noIssue.AddAll("os", "Stdout", "Stderr") return rule, []ast.Node{(*ast.CallExpr)(nil)} } diff --git a/_linters/src/github.com/securego/gosec/rules/ssrf.go b/_linters/src/github.com/securego/gosec/rules/ssrf.go new file mode 100644 index 00000000..9be9b404 --- /dev/null +++ b/_linters/src/github.com/securego/gosec/rules/ssrf.go @@ -0,0 +1,59 @@ +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 + } + } + 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/_linters/src/github.com/securego/gosec/testutils/source.go b/_linters/src/github.com/securego/gosec/testutils/source.go index d3464b48..e0a68342 100644 --- a/_linters/src/github.com/securego/gosec/testutils/source.go +++ b/_linters/src/github.com/securego/gosec/testutils/source.go @@ -192,6 +192,43 @@ 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}} // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ {` @@ -500,7 +537,7 @@ import ( func main() { http.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) { - title := r.URL.Query().Get("title") + title := r.URL.Query().Get("title") f, err := os.Open(title) if err != nil { fmt.Printf("Error: %v\n", err) @@ -512,6 +549,65 @@ func main() { fmt.Fprintf(w, "%s", body) }) log.Fatal(http.ListenAndServe(":3000", nil)) +}`, 1}, {` +package main + +import ( + "log" + "os" + "io/ioutil" +) + + func main() { + f2 := os.Getenv("tainted_file2") + body, err := ioutil.ReadFile("/tmp/" + f2) + if err != nil { + log.Printf("Error: %v\n", err) + } + log.Print(body) + }`, 1}, {` + package main + + import ( + "bufio" + "fmt" + "os" + "path/filepath" + ) + +func main() { + reader := bufio.NewReader(os.Stdin) + fmt.Print("Please enter file to read: ") + file, _ := reader.ReadString('\n') + file = file[:len(file)-1] + f, err := os.Open(filepath.Join("/tmp/service/", file)) + if err != nil { + fmt.Printf("Error: %v\n", err) + } + contents := make([]byte, 15) + if _, err = f.Read(contents); err != nil { + fmt.Printf("Error: %v\n", err) + } + fmt.Println(string(contents)) +}`, 1}, {` +package main + +import ( + "log" + "os" + "io/ioutil" + "path/filepath" +) + +func main() { + dir := os.Getenv("server_root") + f3 := os.Getenv("tainted_file3") + // edge case where both a binary expression and file Join are used. + body, err := ioutil.ReadFile(filepath.Join("/var/"+dir, f3)) + if err != nil { + log.Printf("Error: %v\n", err) + } + log.Print(body) }`, 1}} // SampleCodeG305 - File path traversal when extracting zip archives diff --git a/_linters/src/manifest b/_linters/src/manifest index 89e56eda..11a30409 100644 --- a/_linters/src/manifest +++ b/_linters/src/manifest @@ -142,7 +142,7 @@ "importpath": "github.com/securego/gosec", "repository": "https://github.com/securego/gosec", "vcs": "git", - "revision": "a7cff91312e4ea979b7ec590792a289b9d199391", + "revision": "145f1a0bf413089fb73a26c7859887afe39aadfa", "branch": "master", "notests": true },