From 712de1c63d9db97c81af68cd0dc4372c53d2e57a Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 15 Sep 2020 18:05:35 +0200 Subject: vendor/github.com/golangci/golangci-lint: update to v1.31 --- vendor/github.com/securego/gosec/v2/rules/sql.go | 132 ++++++++++++++++++----- 1 file changed, 108 insertions(+), 24 deletions(-) (limited to 'vendor/github.com/securego/gosec/v2/rules/sql.go') diff --git a/vendor/github.com/securego/gosec/v2/rules/sql.go b/vendor/github.com/securego/gosec/v2/rules/sql.go index 3279a3400..127dec504 100644 --- a/vendor/github.com/securego/gosec/v2/rules/sql.go +++ b/vendor/github.com/securego/gosec/v2/rules/sql.go @@ -17,12 +17,14 @@ package rules import ( "go/ast" "regexp" + "strings" "github.com/securego/gosec/v2" ) type sqlStatement struct { gosec.MetaData + gosec.CallList // Contains a list of patterns which must all match for the rule to match. patterns []*regexp.Regexp @@ -65,33 +67,65 @@ func (s *sqlStrConcat) checkObject(n *ast.Ident, c *gosec.Context) bool { return false } -// Look for "SELECT * FROM table WHERE " + " ' OR 1=1" -func (s *sqlStrConcat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { - if node, ok := n.(*ast.BinaryExpr); ok { - if start, ok := node.X.(*ast.BasicLit); ok { +// checkQuery verifies if the query parameters is a string concatenation +func (s *sqlStrConcat) checkQuery(call *ast.CallExpr, ctx *gosec.Context) (*gosec.Issue, error) { + _, fnName, err := gosec.GetCallInfo(call, ctx) + if err != nil { + return nil, err + } + var query ast.Node + if strings.HasSuffix(fnName, "Context") { + query = call.Args[1] + } else { + query = call.Args[0] + } + + if be, ok := query.(*ast.BinaryExpr); ok { + operands := gosec.GetBinaryExprOperands(be) + if start, ok := operands[0].(*ast.BasicLit); ok { if str, e := gosec.GetString(start); e == nil { if !s.MatchPatterns(str) { return nil, nil } - if _, ok := node.Y.(*ast.BasicLit); ok { - return nil, nil // string cat OK + } + for _, op := range operands[1:] { + if _, ok := op.(*ast.BasicLit); ok { + continue } - if second, ok := node.Y.(*ast.Ident); ok && s.checkObject(second, c) { - return nil, nil + if op, ok := op.(*ast.Ident); ok && s.checkObject(op, ctx) { + continue } - return gosec.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), nil + return gosec.NewIssue(ctx, be, s.ID(), s.What, s.Severity, s.Confidence), nil } } } + + return nil, nil +} + +// Checks SQL query concatenation issues such as "SELECT * FROM table WHERE " + " ' OR 1=1" +func (s *sqlStrConcat) Match(n ast.Node, ctx *gosec.Context) (*gosec.Issue, error) { + switch stmt := n.(type) { + case *ast.AssignStmt: + for _, expr := range stmt.Rhs { + if sqlQueryCall, ok := expr.(*ast.CallExpr); ok && s.ContainsCallExpr(expr, ctx) != nil { + return s.checkQuery(sqlQueryCall, ctx) + } + } + case *ast.ExprStmt: + if sqlQueryCall, ok := stmt.X.(*ast.CallExpr); ok && s.ContainsCallExpr(stmt.X, ctx) != nil { + return s.checkQuery(sqlQueryCall, ctx) + } + } return nil, nil } // NewSQLStrConcat looks for cases where we are building SQL strings via concatenation func NewSQLStrConcat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { - return &sqlStrConcat{ + rule := &sqlStrConcat{ sqlStatement: sqlStatement{ patterns: []*regexp.Regexp{ - regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), + regexp.MustCompile(`(?i)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), }, MetaData: gosec.MetaData{ ID: id, @@ -99,13 +133,19 @@ func NewSQLStrConcat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { Confidence: gosec.High, What: "SQL string concatenation", }, + CallList: gosec.NewCallList(), }, - }, []ast.Node{(*ast.BinaryExpr)(nil)} + } + + rule.AddAll("*database/sql.DB", "Query", "QueryContext", "QueryRow", "QueryRowContext") + rule.AddAll("*database/sql.Tx", "Query", "QueryContext", "QueryRow", "QueryRowContext") + return rule, []ast.Node{(*ast.AssignStmt)(nil), (*ast.ExprStmt)(nil)} } type sqlStrFormat struct { + gosec.CallList sqlStatement - calls gosec.CallList + fmtCalls gosec.CallList noIssue gosec.CallList noIssueQuoted gosec.CallList } @@ -130,14 +170,37 @@ func (s *sqlStrFormat) constObject(e ast.Expr, c *gosec.Context) bool { return false } -// Looks for "fmt.Sprintf("SELECT * FROM foo where '%s', userInput)" -func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) { +func (s *sqlStrFormat) checkQuery(call *ast.CallExpr, ctx *gosec.Context) (*gosec.Issue, error) { + _, fnName, err := gosec.GetCallInfo(call, ctx) + if err != nil { + return nil, err + } + var query ast.Node + if strings.HasSuffix(fnName, "Context") { + query = call.Args[1] + } else { + query = call.Args[0] + } + + if ident, ok := query.(*ast.Ident); ok && ident.Obj != nil { + decl := ident.Obj.Decl + if assign, ok := decl.(*ast.AssignStmt); ok { + for _, expr := range assign.Rhs { + issue, err := s.checkFormatting(expr, ctx) + if issue != nil { + return issue, err + } + } + } + } + return nil, nil +} + +func (s *sqlStrFormat) checkFormatting(n ast.Node, ctx *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.ContainsPkgCallExpr(n, c, false); node != nil { + if node := s.fmtCalls.ContainsPkgCallExpr(n, ctx, false); 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" { @@ -177,7 +240,7 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) if argIndex+1 < len(node.Args) { allSafe := true for _, arg := range node.Args[argIndex+1:] { - if n := s.noIssueQuoted.ContainsPkgCallExpr(arg, c, true); n == nil && !s.constObject(arg, c) { + if n := s.noIssueQuoted.ContainsPkgCallExpr(arg, ctx, true); n == nil && !s.constObject(arg, ctx) { allSafe = false break } @@ -187,7 +250,24 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) } } if s.MatchPatterns(formatter) { - return gosec.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), nil + return gosec.NewIssue(ctx, n, s.ID(), s.What, s.Severity, s.Confidence), nil + } + } + return nil, nil +} + +// Check SQL query formatting issues such as "fmt.Sprintf("SELECT * FROM foo where '%s', userInput)" +func (s *sqlStrFormat) Match(n ast.Node, ctx *gosec.Context) (*gosec.Issue, error) { + switch stmt := n.(type) { + case *ast.AssignStmt: + for _, expr := range stmt.Rhs { + if sqlQueryCall, ok := expr.(*ast.CallExpr); ok && s.ContainsCallExpr(expr, ctx) != nil { + return s.checkQuery(sqlQueryCall, ctx) + } + } + case *ast.ExprStmt: + if sqlQueryCall, ok := stmt.X.(*ast.CallExpr); ok && s.ContainsCallExpr(stmt.X, ctx) != nil { + return s.checkQuery(sqlQueryCall, ctx) } } return nil, nil @@ -196,12 +276,13 @@ 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(), + CallList: gosec.NewCallList(), + fmtCalls: gosec.NewCallList(), noIssue: gosec.NewCallList(), noIssueQuoted: gosec.NewCallList(), sqlStatement: sqlStatement{ patterns: []*regexp.Regexp{ - regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), + regexp.MustCompile("(?i)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), regexp.MustCompile("%[^bdoxXfFp]"), }, MetaData: gosec.MetaData{ @@ -212,8 +293,11 @@ func NewSQLStrFormat(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { }, }, } - rule.calls.AddAll("fmt", "Sprint", "Sprintf", "Sprintln", "Fprintf") + rule.AddAll("*database/sql.DB", "Query", "QueryContext", "QueryRow", "QueryRowContext") + rule.AddAll("*database/sql.Tx", "Query", "QueryContext", "QueryRow", "QueryRowContext") + rule.fmtCalls.AddAll("fmt", "Sprint", "Sprintf", "Sprintln", "Fprintf") rule.noIssue.AddAll("os", "Stdout", "Stderr") rule.noIssueQuoted.Add("github.com/lib/pq", "QuoteIdentifier") - return rule, []ast.Node{(*ast.CallExpr)(nil)} + + return rule, []ast.Node{(*ast.AssignStmt)(nil), (*ast.ExprStmt)(nil)} } -- cgit mrf-deployment