mirror of
https://github.com/securego/gosec.git
synced 2024-11-05 19:45:51 +00:00
Merge pull request #138 from jonmcclintock/sqli-format-whitelist
Adjust SQL format-string rules to ignore inherently safe formats
This commit is contained in:
commit
51b4a4ddc8
2 changed files with 47 additions and 6 deletions
30
rules/sql.go
30
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,
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue