diff --git a/rules/sql.go b/rules/sql.go index 602f0ab..c6505e3 100644 --- a/rules/sql.go +++ b/rules/sql.go @@ -23,7 +23,19 @@ import ( type sqlStatement struct { gas.MetaData - pattern *regexp.Regexp + + // Contains a list of patterns which must all match for the rule to match. + patterns []*regexp.Regexp +} + +// See if the string matches the patterns for the statement. +func (s sqlStatement) MatchPatterns(str string) bool { + for _, pattern := range s.patterns { + if !pattern.MatchString(str) { + return false + } + } + return true } type sqlStrConcat struct { @@ -42,7 +54,10 @@ func (s *sqlStrConcat) checkObject(n *ast.Ident) bool { func (s *sqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node, ok := n.(*ast.BinaryExpr); ok { if start, ok := node.X.(*ast.BasicLit); ok { - if str, e := gas.GetString(start); s.pattern.MatchString(str) && e == nil { + if str, e := gas.GetString(start); e == nil { + if !s.MatchPatterns(str) { + return nil, nil + } if _, ok := node.Y.(*ast.BasicLit); ok { return nil, nil // string cat OK } @@ -60,7 +75,9 @@ func (s *sqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { func NewSQLStrConcat(conf gas.Config) (gas.Rule, []ast.Node) { return &sqlStrConcat{ sqlStatement: sqlStatement{ - pattern: regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), + patterns: []*regexp.Regexp{ + regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), + }, MetaData: gas.MetaData{ Severity: gas.Medium, Confidence: gas.High, @@ -80,7 +97,7 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { // TODO(gm) improve confidence if database/sql is being used if node := s.calls.ContainsCallExpr(n, c); node != nil { - if arg, e := gas.GetString(node.Args[0]); s.pattern.MatchString(arg) && e == nil { + if arg, e := gas.GetString(node.Args[0]); s.MatchPatterns(arg) && e == nil { return gas.NewIssue(c, n, s.What, s.Severity, s.Confidence), nil } } @@ -92,7 +109,10 @@ func NewSQLStrFormat(conf gas.Config) (gas.Rule, []ast.Node) { rule := &sqlStrFormat{ calls: gas.NewCallList(), sqlStatement: sqlStatement{ - pattern: regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), + patterns: []*regexp.Regexp{ + regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), + regexp.MustCompile("%[^bdoxXfFp]"), + }, MetaData: gas.MetaData{ Severity: gas.Medium, Confidence: gas.High, diff --git a/testutils/source.go b/testutils/source.go index 7eb0130..f606e4a 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -206,7 +206,28 @@ func main(){ panic(err) } defer rows.Close() -}`, 1}, { +}`, 1}, {` +// Format string false positive, safe string spec. +package main +import ( + "database/sql" + "fmt" + "os" + //_ "github.com/mattn/go-sqlite3" +) + +func main(){ + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + panic(err) + } + q := fmt.Sprintf("SELECT * FROM foo where id = %d", os.Args[1]) + rows, err := db.Query(q) + if err != nil { + panic(err) + } + defer rows.Close() +}`, 0}, { ` // Format string false positive package main