Skip to content

Commit

Permalink
Better SQLi testing
Browse files Browse the repository at this point in the history
This prevents the string concat tests flagging a false positive if
joining two literal strings (eg "SELECT * FROM " + " table" ... )
or with a constant (eg const tab = "name"; "SELECT * from " + tab)
  • Loading branch information
Tim Kelsey committed Jul 27, 2016
1 parent 2d0a26d commit 3e4d96e
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 8 deletions.
28 changes: 21 additions & 7 deletions rules/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
package rules

import (
gas "github.com/HewlettPackard/gas/core"
"go/ast"
"reflect"
"regexp"

gas "github.com/HewlettPackard/gas/core"
)

type SqlStatement struct {
Expand All @@ -30,13 +30,27 @@ type SqlStrConcat struct {
SqlStatement
}

// see if we can figgure out what it is
func (s *SqlStrConcat) checkObject(n *ast.Ident) bool {
if n.Obj != nil {
return (n.Obj.Kind != ast.Var || n.Obj.Kind != ast.Fun)
}
return false
}

// Look for "SELECT * FROM table WHERE " + " ' OR 1=1"
func (s *SqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) {
a := reflect.TypeOf(&ast.BinaryExpr{})
b := reflect.TypeOf(&ast.BasicLit{})
if node := gas.SimpleSelect(n, a, b); node != nil {
if str, _ := gas.GetString(node); s.pattern.MatchString(str) {
return gas.NewIssue(c, n, s.What, s.Severity, s.Confidence), nil
if node, ok := n.(*ast.BinaryExpr); ok {
if start, ok := node.X.(*ast.BasicLit); ok {
if str, _ := gas.GetString(start); s.pattern.MatchString(str) {
if _, ok := node.Y.(*ast.BasicLit); ok {
return nil, nil // string cat OK
}
if second, ok := node.Y.(*ast.Ident); ok && s.checkObject(second) {
return nil, nil
}
return gas.NewIssue(c, n, s.What, s.Severity, s.Confidence), nil
}
}
}
return nil, nil
Expand Down
74 changes: 73 additions & 1 deletion rules/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
package rules

import (
gas "github.com/HewlettPackard/gas/core"
"testing"

gas "github.com/HewlettPackard/gas/core"
)

func TestSQLInjectionViaConcatenation(t *testing.T) {
Expand Down Expand Up @@ -144,3 +145,74 @@ func TestSQLInjectionFalsePositiveB(t *testing.T) {

checkTestResults(t, issues, 0, "Not expected to match")
}

func TestSQLInjectionFalsePositiveC(t *testing.T) {
analyzer := gas.NewAnalyzer(false, nil)
analyzer.AddRule(NewSqlStrConcat())
analyzer.AddRule(NewSqlStrFormat())

source := `
package main
import (
"database/sql"
"fmt"
"os"
_ "github.com/mattn/go-sqlite3"
)
var staticQuery = "SELECT * FROM foo WHERE age < "
func main(){
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
rows, err := db.Query(staticQuery + "32")
if err != nil {
panic(err)
}
defer rows.Close()
}
`
issues := gasTestRunner(source, analyzer)

checkTestResults(t, issues, 0, "Not expected to match")
}

func TestSQLInjectionFalsePositiveD(t *testing.T) {
analyzer := gas.NewAnalyzer(false, nil)
analyzer.AddRule(NewSqlStrConcat())
analyzer.AddRule(NewSqlStrFormat())

source := `
package main
import (
"database/sql"
"fmt"
"os"
_ "github.com/mattn/go-sqlite3"
)
const age = "32"
var staticQuery = "SELECT * FROM foo WHERE age < "
func main(){
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
rows, err := db.Query(staticQuery + age)
if err != nil {
panic(err)
}
defer rows.Close()
}
`
issues := gasTestRunner(source, analyzer)

checkTestResults(t, issues, 0, "Not expected to match")
}

0 comments on commit 3e4d96e

Please sign in to comment.