From 7fac1a42f90684ed9fe066d9b7b5eced7d79e9c5 Mon Sep 17 00:00:00 2001 From: Kent Gruber Date: Fri, 4 Jul 2025 11:55:12 -0400 Subject: [PATCH 1/5] Add tests and support for additional SQL packages --- README.md | 14 ++ sql/injection/injection.go | 179 +++++++++++++++++- sql/injection/injection_test.go | 32 ++++ .../github.com/Masterminds/squirrel/mock.go | 5 + .../src/github.com/go-gorm/gorm/mock.go | 25 +++ .../testdata/src/github.com/go-pg/pg/mock.go | 25 +++ .../src/github.com/jmoiron/sqlx/mock.go | 30 +++ .../src/github.com/lann/squirrel/mock.go | 5 + .../src/github.com/mattn/go-sqlite3/mock.go | 3 + .../github.com/raindog308/gorqlite/mock.go | 10 + .../src/github.com/rqlite/gorqlite/mock.go | 10 + .../gopkg.in/Masterminds/squirrel.v1/mock.go | 5 + .../src/gorm.io/driver/sqlite/mock.go | 7 + .../testdata/src/gorm.io/gorm/mock.go | 25 +++ sql/injection/testdata/src/n/main.go | 28 +++ sql/injection/testdata/src/o/main.go | 15 ++ sql/injection/testdata/src/p/main.go | 21 ++ sql/injection/testdata/src/q/main.go | 19 ++ sql/injection/testdata/src/r/main.go | 19 ++ sql/injection/testdata/src/s/main.go | 16 ++ sql/injection/testdata/src/t/main.go | 24 +++ sql/injection/testdata/src/u/main.go | 16 ++ .../testdata/src/xorm.io/xorm/mock.go | 41 ++++ 23 files changed, 565 insertions(+), 9 deletions(-) create mode 100644 sql/injection/testdata/src/github.com/Masterminds/squirrel/mock.go create mode 100644 sql/injection/testdata/src/github.com/go-gorm/gorm/mock.go create mode 100644 sql/injection/testdata/src/github.com/go-pg/pg/mock.go create mode 100644 sql/injection/testdata/src/github.com/jmoiron/sqlx/mock.go create mode 100644 sql/injection/testdata/src/github.com/lann/squirrel/mock.go create mode 100644 sql/injection/testdata/src/github.com/mattn/go-sqlite3/mock.go create mode 100644 sql/injection/testdata/src/github.com/raindog308/gorqlite/mock.go create mode 100644 sql/injection/testdata/src/github.com/rqlite/gorqlite/mock.go create mode 100644 sql/injection/testdata/src/gopkg.in/Masterminds/squirrel.v1/mock.go create mode 100644 sql/injection/testdata/src/gorm.io/driver/sqlite/mock.go create mode 100644 sql/injection/testdata/src/gorm.io/gorm/mock.go create mode 100644 sql/injection/testdata/src/n/main.go create mode 100644 sql/injection/testdata/src/o/main.go create mode 100644 sql/injection/testdata/src/p/main.go create mode 100644 sql/injection/testdata/src/q/main.go create mode 100644 sql/injection/testdata/src/r/main.go create mode 100644 sql/injection/testdata/src/s/main.go create mode 100644 sql/injection/testdata/src/t/main.go create mode 100644 sql/injection/testdata/src/u/main.go create mode 100644 sql/injection/testdata/src/xorm.io/xorm/mock.go diff --git a/README.md b/README.md index 4e9c0b8..94352d9 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,20 @@ $ go install github.com/picatz/taint/cmd/taint@latest The `sqli` [analyzer](https://pkg.go.dev/golang.org/x/tools/go/analysis#Analyzer) finds potential SQL injections. +Supported SQL packages include: + +- the standard library `database/sql` package +- `github.com/jinzhu/gorm` (GORM v1) +- `gorm.io/gorm` (GORM v2) +- `github.com/jmoiron/sqlx` +- `github.com/go-gorm/gorm` (GORM v2 alt) +- `xorm.io/xorm` and `github.com/go-xorm/xorm` +- `github.com/go-pg/pg` +- `github.com/rqlite/gorqlite` +- `github.com/raindog308/gorqlite` +- `github.com/Masterminds/squirrel` and variants +- database drivers like `github.com/mattn/go-sqlite3` + ```console $ go install github.com/picatz/taint/cmd/sqli@latest ``` diff --git a/sql/injection/injection.go b/sql/injection/injection.go index d1979a1..657f53f 100644 --- a/sql/injection/injection.go +++ b/sql/injection/injection.go @@ -39,7 +39,7 @@ var userControlledValues = taint.NewSources( // // TODO: add more, consider pointer variants and specific fields on types // TODO: consider support for protobuf defined *Request types... - // TODO: consider supprot for gRPC request metadata (HTTP2 headers) + // TODO: consider support for gRPC request metadata (HTTP2 headers) // TODO: consider support for msgpack-rpc? ) @@ -68,6 +68,139 @@ var injectableSQLMethods = taint.NewSinks( "(*github.com/jinzhu/gorm.DB).Raw", "(*github.com/jinzhu/gorm.DB).Exec", "(*github.com/jinzhu/gorm.DB).Order", + // GORM v2 + "(*gorm.io/gorm.DB).Where", + "(*gorm.io/gorm.DB).Or", + "(*gorm.io/gorm.DB).Not", + "(*gorm.io/gorm.DB).Group", + "(*gorm.io/gorm.DB).Having", + "(*gorm.io/gorm.DB).Joins", + "(*gorm.io/gorm.DB).Select", + "(*gorm.io/gorm.DB).Distinct", + "(*gorm.io/gorm.DB).Pluck", + "(*gorm.io/gorm.DB).Raw", + "(*gorm.io/gorm.DB).Exec", + "(*gorm.io/gorm.DB).Order", + // Alternative GORM v2 import path + "(*github.com/go-gorm/gorm.DB).Where", + "(*github.com/go-gorm/gorm.DB).Or", + "(*github.com/go-gorm/gorm.DB).Not", + "(*github.com/go-gorm/gorm.DB).Group", + "(*github.com/go-gorm/gorm.DB).Having", + "(*github.com/go-gorm/gorm.DB).Joins", + "(*github.com/go-gorm/gorm.DB).Select", + "(*github.com/go-gorm/gorm.DB).Distinct", + "(*github.com/go-gorm/gorm.DB).Pluck", + "(*github.com/go-gorm/gorm.DB).Raw", + "(*github.com/go-gorm/gorm.DB).Exec", + "(*github.com/go-gorm/gorm.DB).Order", + // sqlx + "(*github.com/jmoiron/sqlx.DB).Queryx", + "(*github.com/jmoiron/sqlx.DB).QueryRowx", + "(*github.com/jmoiron/sqlx.DB).Query", + "(*github.com/jmoiron/sqlx.DB).QueryRow", + "(*github.com/jmoiron/sqlx.DB).Select", + "(*github.com/jmoiron/sqlx.DB).Get", + "(*github.com/jmoiron/sqlx.DB).Exec", + "(*github.com/jmoiron/sqlx.Tx).Queryx", + "(*github.com/jmoiron/sqlx.Tx).QueryRowx", + "(*github.com/jmoiron/sqlx.Tx).Query", + "(*github.com/jmoiron/sqlx.Tx).QueryRow", + "(*github.com/jmoiron/sqlx.Tx).Select", + "(*github.com/jmoiron/sqlx.Tx).Get", + "(*github.com/jmoiron/sqlx.Tx).Exec", + // xorm + "(*xorm.io/xorm.Engine).Query", + "(*xorm.io/xorm.Engine).Exec", + "(*xorm.io/xorm.Engine).QueryString", + "(*xorm.io/xorm.Engine).QueryInterface", + "(*xorm.io/xorm.Engine).SQL", + "(*xorm.io/xorm.Engine).Where", + "(*xorm.io/xorm.Engine).And", + "(*xorm.io/xorm.Engine).Or", + "(*xorm.io/xorm.Engine).Alias", + "(*xorm.io/xorm.Engine).NotIn", + "(*xorm.io/xorm.Engine).In", + "(*xorm.io/xorm.Engine).Select", + "(*xorm.io/xorm.Engine).SetExpr", + "(*xorm.io/xorm.Engine).OrderBy", + "(*xorm.io/xorm.Engine).Having", + "(*xorm.io/xorm.Engine).GroupBy", + "(*xorm.io/xorm.Engine).Join", + "(*xorm.io/xorm.Session).Query", + "(*xorm.io/xorm.Session).Exec", + "(*xorm.io/xorm.Session).QueryString", + "(*xorm.io/xorm.Session).QueryInterface", + "(*xorm.io/xorm.Session).SQL", + "(*xorm.io/xorm.Session).Where", + "(*xorm.io/xorm.Session).And", + "(*xorm.io/xorm.Session).Or", + "(*xorm.io/xorm.Session).Alias", + "(*xorm.io/xorm.Session).NotIn", + "(*xorm.io/xorm.Session).In", + "(*xorm.io/xorm.Session).Select", + "(*xorm.io/xorm.Session).SetExpr", + "(*xorm.io/xorm.Session).OrderBy", + "(*xorm.io/xorm.Session).Having", + "(*xorm.io/xorm.Session).GroupBy", + "(*xorm.io/xorm.Session).Join", + // Alternative xorm import path + "(*github.com/go-xorm/xorm.Engine).Query", + "(*github.com/go-xorm/xorm.Engine).Exec", + "(*github.com/go-xorm/xorm.Engine).QueryString", + "(*github.com/go-xorm/xorm.Engine).QueryInterface", + "(*github.com/go-xorm/xorm.Engine).SQL", + "(*github.com/go-xorm/xorm.Engine).Where", + "(*github.com/go-xorm/xorm.Engine).And", + "(*github.com/go-xorm/xorm.Engine).Or", + "(*github.com/go-xorm/xorm.Engine).Alias", + "(*github.com/go-xorm/xorm.Engine).NotIn", + "(*github.com/go-xorm/xorm.Engine).In", + "(*github.com/go-xorm/xorm.Engine).Select", + "(*github.com/go-xorm/xorm.Engine).SetExpr", + "(*github.com/go-xorm/xorm.Engine).OrderBy", + "(*github.com/go-xorm/xorm.Engine).Having", + "(*github.com/go-xorm/xorm.Engine).GroupBy", + "(*github.com/go-xorm/xorm.Engine).Join", + "(*github.com/go-xorm/xorm.Session).Query", + "(*github.com/go-xorm/xorm.Session).Exec", + "(*github.com/go-xorm/xorm.Session).QueryString", + "(*github.com/go-xorm/xorm.Session).QueryInterface", + "(*github.com/go-xorm/xorm.Session).SQL", + "(*github.com/go-xorm/xorm.Session).Where", + "(*github.com/go-xorm/xorm.Session).And", + "(*github.com/go-xorm/xorm.Session).Or", + "(*github.com/go-xorm/xorm.Session).Alias", + "(*github.com/go-xorm/xorm.Session).NotIn", + "(*github.com/go-xorm/xorm.Session).In", + "(*github.com/go-xorm/xorm.Session).Select", + "(*github.com/go-xorm/xorm.Session).SetExpr", + "(*github.com/go-xorm/xorm.Session).OrderBy", + "(*github.com/go-xorm/xorm.Session).Having", + "(*github.com/go-xorm/xorm.Session).GroupBy", + "(*github.com/go-xorm/xorm.Session).Join", + // go-pg + "(*github.com/go-pg/pg.DB).Query", + "(*github.com/go-pg/pg.DB).QueryOne", + "(*github.com/go-pg/pg.DB).Exec", + "(*github.com/go-pg/pg.DB).ExecOne", + "(*github.com/go-pg/pg.Tx).Query", + "(*github.com/go-pg/pg.Tx).QueryOne", + "(*github.com/go-pg/pg.Tx).Exec", + "(*github.com/go-pg/pg.Tx).ExecOne", + // rqlite + "(*github.com/rqlite/gorqlite.Connection).Query", + "(*github.com/rqlite/gorqlite.Connection).QueryOne", + "(*github.com/rqlite/gorqlite.Connection).Write", + "(*github.com/rqlite/gorqlite.Connection).WriteOne", + "(*github.com/raindog308/gorqlite.Connection).Query", + "(*github.com/raindog308/gorqlite.Connection).QueryOne", + "(*github.com/raindog308/gorqlite.Connection).Write", + "(*github.com/raindog308/gorqlite.Connection).WriteOne", + // Squirrel + "github.com/Masterminds/squirrel.Expr", + "gopkg.in/Masterminds/squirrel.v1.Expr", + "github.com/lann/squirrel.Expr", // // TODO: add more, consider (non-)pointer variants? ) @@ -99,11 +232,25 @@ func imports(pass *analysis.Pass, pkgs ...string) bool { } func run(pass *analysis.Pass) (interface{}, error) { - // Require the database/sql or GORM v1 packages are imported in the - // program being analyzed before running the analysis. - // - // This prevents wasting time analyzing programs that don't use SQL. - if !imports(pass, "database/sql", "github.com/jinzhu/gorm") { + // Require at least one supported SQL package to be imported before + // running the analysis. This avoids wasting time analyzing programs + // that do not use SQL. + if !imports(pass, + "database/sql", + "github.com/mattn/go-sqlite3", + "github.com/jinzhu/gorm", + "gorm.io/gorm", + "github.com/go-gorm/gorm", + "github.com/jmoiron/sqlx", + "xorm.io/xorm", + "github.com/go-xorm/xorm", + "github.com/go-pg/pg", + "github.com/rqlite/gorqlite", + "github.com/raindog308/gorqlite", + "github.com/Masterminds/squirrel", + "gopkg.in/Masterminds/squirrel.v1", + "github.com/lann/squirrel", + ) { return nil, nil } @@ -132,7 +279,7 @@ func run(pass *analysis.Pass) (interface{}, error) { // Today, I believe the callgraphutil package is the most // accurate, but I'd love to be proven wrong. - // Note: this actually panis for testcase b + // Note: this actually panics for testcase b // ptares, err := pointer.Analyze(&pointer.Config{ // Mains: []*ssa.Package{buildSSA.Pkg}, // BuildCallGraph: true, @@ -170,14 +317,28 @@ func run(pass *analysis.Pass) (interface{}, error) { // (first argument after context). queryEdge := result.Path[len(result.Path)-1] - // Get the query arguments, skipping the first element, pointer to the DB. - queryArgs := queryEdge.Site.Common().Args[1:] + // Get the query arguments. If the sink is a method call, the + // first argument is the receiver, which we skip. + queryArgs := queryEdge.Site.Common().Args + if queryEdge.Site.Common().Signature().Recv() != nil { + if len(queryArgs) < 1 { + continue + } + queryArgs = queryArgs[1:] + } // Skip the context argument, if using a *Context query variant. if strings.HasPrefix(queryEdge.Site.Value().Call.Value.String(), "Context") { + if len(queryArgs) < 2 { + continue + } queryArgs = queryArgs[1:] } + if len(queryArgs) == 0 { + continue + } + // Get the query function parameter. query := queryArgs[0] diff --git a/sql/injection/injection_test.go b/sql/injection/injection_test.go index e638c36..b61a178 100644 --- a/sql/injection/injection_test.go +++ b/sql/injection/injection_test.go @@ -65,3 +65,35 @@ func TestL(t *testing.T) { func TestM(t *testing.T) { analysistest.Run(t, testdata, Analyzer, "m") } + +func TestN(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "n") +} + +func TestO(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "o") +} + +func TestP(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "p") +} + +func TestQ(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "q") +} + +func TestR(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "r") +} + +func TestS(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "s") +} + +func TestT(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "t") +} + +func TestU(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "u") +} diff --git a/sql/injection/testdata/src/github.com/Masterminds/squirrel/mock.go b/sql/injection/testdata/src/github.com/Masterminds/squirrel/mock.go new file mode 100644 index 0000000..6888b6d --- /dev/null +++ b/sql/injection/testdata/src/github.com/Masterminds/squirrel/mock.go @@ -0,0 +1,5 @@ +package squirrel + +type Sqlizer interface{} + +func Expr(sql string, args ...interface{}) Sqlizer { return nil } diff --git a/sql/injection/testdata/src/github.com/go-gorm/gorm/mock.go b/sql/injection/testdata/src/github.com/go-gorm/gorm/mock.go new file mode 100644 index 0000000..3c3c1e1 --- /dev/null +++ b/sql/injection/testdata/src/github.com/go-gorm/gorm/mock.go @@ -0,0 +1,25 @@ +package gorm + +type Model struct{} + +type DB struct{} + +type Config struct{} + +type Dialector interface{} + +func Open(d Dialector, config *Config) (*DB, error) { return nil, nil } + +func (s *DB) Where(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Or(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Not(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Group(query string) *DB { return nil } +func (s *DB) Having(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Joins(query string, args ...interface{}) *DB { return nil } +func (s *DB) Select(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Distinct(args ...interface{}) *DB { return nil } +func (s *DB) Pluck(column string, value interface{}) *DB { return nil } +func (s *DB) Raw(sql string, values ...interface{}) *DB { return nil } +func (s *DB) Exec(sql string, values ...interface{}) *DB { return nil } +func (s *DB) Order(value interface{}) *DB { return nil } +func (s *DB) Find(dest interface{}, conds ...interface{}) *DB { return nil } diff --git a/sql/injection/testdata/src/github.com/go-pg/pg/mock.go b/sql/injection/testdata/src/github.com/go-pg/pg/mock.go new file mode 100644 index 0000000..89fb7c2 --- /dev/null +++ b/sql/injection/testdata/src/github.com/go-pg/pg/mock.go @@ -0,0 +1,25 @@ +package pg + +type DB struct{} + +type Tx struct{} + +type Result struct{} + +func (db *DB) Query(dest interface{}, query string, params ...interface{}) (Result, error) { + return Result{}, nil +} +func (db *DB) QueryOne(dest interface{}, query string, params ...interface{}) (Result, error) { + return Result{}, nil +} +func (db *DB) Exec(query string, params ...interface{}) (Result, error) { return Result{}, nil } +func (db *DB) ExecOne(query string, params ...interface{}) (Result, error) { return Result{}, nil } + +func (tx *Tx) Query(dest interface{}, query string, params ...interface{}) (Result, error) { + return Result{}, nil +} +func (tx *Tx) QueryOne(dest interface{}, query string, params ...interface{}) (Result, error) { + return Result{}, nil +} +func (tx *Tx) Exec(query string, params ...interface{}) (Result, error) { return Result{}, nil } +func (tx *Tx) ExecOne(query string, params ...interface{}) (Result, error) { return Result{}, nil } diff --git a/sql/injection/testdata/src/github.com/jmoiron/sqlx/mock.go b/sql/injection/testdata/src/github.com/jmoiron/sqlx/mock.go new file mode 100644 index 0000000..8f1be6b --- /dev/null +++ b/sql/injection/testdata/src/github.com/jmoiron/sqlx/mock.go @@ -0,0 +1,30 @@ +package sqlx + +type DB struct{} + +type Tx struct{} + +type Row struct{} + +type Rows struct{} + +type Result interface{} + +func Open(driverName, dataSourceName string) (*DB, error) { return nil, nil } +func Connect(driverName, dataSourceName string) (*DB, error) { return nil, nil } + +func (db *DB) Query(query string, args ...interface{}) (*Rows, error) { return nil, nil } +func (db *DB) Queryx(query string, args ...interface{}) (*Rows, error) { return nil, nil } +func (db *DB) QueryRow(query string, args ...interface{}) *Row { return nil } +func (db *DB) QueryRowx(query string, args ...interface{}) *Row { return nil } +func (db *DB) Select(dest interface{}, query string, args ...interface{}) error { return nil } +func (db *DB) Get(dest interface{}, query string, args ...interface{}) error { return nil } +func (db *DB) Exec(query string, args ...interface{}) (Result, error) { return nil, nil } + +func (tx *Tx) Query(query string, args ...interface{}) (*Rows, error) { return nil, nil } +func (tx *Tx) Queryx(query string, args ...interface{}) (*Rows, error) { return nil, nil } +func (tx *Tx) QueryRow(query string, args ...interface{}) *Row { return nil } +func (tx *Tx) QueryRowx(query string, args ...interface{}) *Row { return nil } +func (tx *Tx) Select(dest interface{}, query string, args ...interface{}) error { return nil } +func (tx *Tx) Get(dest interface{}, query string, args ...interface{}) error { return nil } +func (tx *Tx) Exec(query string, args ...interface{}) (Result, error) { return nil, nil } diff --git a/sql/injection/testdata/src/github.com/lann/squirrel/mock.go b/sql/injection/testdata/src/github.com/lann/squirrel/mock.go new file mode 100644 index 0000000..6888b6d --- /dev/null +++ b/sql/injection/testdata/src/github.com/lann/squirrel/mock.go @@ -0,0 +1,5 @@ +package squirrel + +type Sqlizer interface{} + +func Expr(sql string, args ...interface{}) Sqlizer { return nil } diff --git a/sql/injection/testdata/src/github.com/mattn/go-sqlite3/mock.go b/sql/injection/testdata/src/github.com/mattn/go-sqlite3/mock.go new file mode 100644 index 0000000..9c79269 --- /dev/null +++ b/sql/injection/testdata/src/github.com/mattn/go-sqlite3/mock.go @@ -0,0 +1,3 @@ +package sqlite3 + +func init() {} diff --git a/sql/injection/testdata/src/github.com/raindog308/gorqlite/mock.go b/sql/injection/testdata/src/github.com/raindog308/gorqlite/mock.go new file mode 100644 index 0000000..fef9656 --- /dev/null +++ b/sql/injection/testdata/src/github.com/raindog308/gorqlite/mock.go @@ -0,0 +1,10 @@ +package gorqlite + +type Connection struct{} + +func Open(addr string) (*Connection, error) { return nil, nil } + +func (c *Connection) Query(dest interface{}, query string, args ...interface{}) error { return nil } +func (c *Connection) QueryOne(dest interface{}, query string, args ...interface{}) error { return nil } +func (c *Connection) Write(query string, args ...interface{}) error { return nil } +func (c *Connection) WriteOne(query string, args ...interface{}) error { return nil } diff --git a/sql/injection/testdata/src/github.com/rqlite/gorqlite/mock.go b/sql/injection/testdata/src/github.com/rqlite/gorqlite/mock.go new file mode 100644 index 0000000..fef9656 --- /dev/null +++ b/sql/injection/testdata/src/github.com/rqlite/gorqlite/mock.go @@ -0,0 +1,10 @@ +package gorqlite + +type Connection struct{} + +func Open(addr string) (*Connection, error) { return nil, nil } + +func (c *Connection) Query(dest interface{}, query string, args ...interface{}) error { return nil } +func (c *Connection) QueryOne(dest interface{}, query string, args ...interface{}) error { return nil } +func (c *Connection) Write(query string, args ...interface{}) error { return nil } +func (c *Connection) WriteOne(query string, args ...interface{}) error { return nil } diff --git a/sql/injection/testdata/src/gopkg.in/Masterminds/squirrel.v1/mock.go b/sql/injection/testdata/src/gopkg.in/Masterminds/squirrel.v1/mock.go new file mode 100644 index 0000000..6888b6d --- /dev/null +++ b/sql/injection/testdata/src/gopkg.in/Masterminds/squirrel.v1/mock.go @@ -0,0 +1,5 @@ +package squirrel + +type Sqlizer interface{} + +func Expr(sql string, args ...interface{}) Sqlizer { return nil } diff --git a/sql/injection/testdata/src/gorm.io/driver/sqlite/mock.go b/sql/injection/testdata/src/gorm.io/driver/sqlite/mock.go new file mode 100644 index 0000000..d414200 --- /dev/null +++ b/sql/injection/testdata/src/gorm.io/driver/sqlite/mock.go @@ -0,0 +1,7 @@ +package sqlite + +type dialector struct{} + +type Dialector = dialector + +func Open(dsn string) Dialector { return dialector{} } diff --git a/sql/injection/testdata/src/gorm.io/gorm/mock.go b/sql/injection/testdata/src/gorm.io/gorm/mock.go new file mode 100644 index 0000000..3c3c1e1 --- /dev/null +++ b/sql/injection/testdata/src/gorm.io/gorm/mock.go @@ -0,0 +1,25 @@ +package gorm + +type Model struct{} + +type DB struct{} + +type Config struct{} + +type Dialector interface{} + +func Open(d Dialector, config *Config) (*DB, error) { return nil, nil } + +func (s *DB) Where(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Or(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Not(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Group(query string) *DB { return nil } +func (s *DB) Having(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Joins(query string, args ...interface{}) *DB { return nil } +func (s *DB) Select(query interface{}, args ...interface{}) *DB { return nil } +func (s *DB) Distinct(args ...interface{}) *DB { return nil } +func (s *DB) Pluck(column string, value interface{}) *DB { return nil } +func (s *DB) Raw(sql string, values ...interface{}) *DB { return nil } +func (s *DB) Exec(sql string, values ...interface{}) *DB { return nil } +func (s *DB) Order(value interface{}) *DB { return nil } +func (s *DB) Find(dest interface{}, conds ...interface{}) *DB { return nil } diff --git a/sql/injection/testdata/src/n/main.go b/sql/injection/testdata/src/n/main.go new file mode 100644 index 0000000..2606f11 --- /dev/null +++ b/sql/injection/testdata/src/n/main.go @@ -0,0 +1,28 @@ +package main + +import ( + "fmt" + "net/http" + + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +type User struct { + gorm.Model + Name string +} + +func handle(db *gorm.DB, u string) { + q := fmt.Sprintf("name='%s'", u) + var users []User + db.Where(q).Find(&users) // want "potential sql injection" +} + +func main() { + db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + handle(db, r.URL.Query().Get("u")) + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/o/main.go b/sql/injection/testdata/src/o/main.go new file mode 100644 index 0000000..7388e00 --- /dev/null +++ b/sql/injection/testdata/src/o/main.go @@ -0,0 +1,15 @@ +package main + +import ( + "net/http" + + "github.com/jmoiron/sqlx" +) + +func main() { + db, _ := sqlx.Open("sqlite3", ":memory:") + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + db.Queryx(r.URL.Query().Get("q")) // want "potential sql injection" + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/p/main.go b/sql/injection/testdata/src/p/main.go new file mode 100644 index 0000000..e702f7f --- /dev/null +++ b/sql/injection/testdata/src/p/main.go @@ -0,0 +1,21 @@ +package main + +import ( + "net/http" + + "xorm.io/xorm" +) + +func business(db *xorm.Engine, q string) { + db.Query(q) // want "potential sql injection" + db.Where(q) // want "potential sql injection" + db.SQL(q) // want "potential sql injection" +} + +func main() { + var db *xorm.Engine + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + business(db, r.URL.Query().Get("q")) + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/q/main.go b/sql/injection/testdata/src/q/main.go new file mode 100644 index 0000000..62473ee --- /dev/null +++ b/sql/injection/testdata/src/q/main.go @@ -0,0 +1,19 @@ +package main + +import ( + "net/http" + + "github.com/go-pg/pg" +) + +func business(db *pg.DB, q string) { + db.Exec(q) // want "potential sql injection" +} + +func main() { + var db *pg.DB + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + business(db, r.URL.Query().Get("q")) + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/r/main.go b/sql/injection/testdata/src/r/main.go new file mode 100644 index 0000000..49749e4 --- /dev/null +++ b/sql/injection/testdata/src/r/main.go @@ -0,0 +1,19 @@ +package main + +import ( + "net/http" + + "github.com/rqlite/gorqlite" +) + +func business(conn *gorqlite.Connection, q string) { + conn.Write(q) // want "potential sql injection" +} + +func main() { + var c *gorqlite.Connection + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + business(c, r.URL.Query().Get("q")) + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/s/main.go b/sql/injection/testdata/src/s/main.go new file mode 100644 index 0000000..c1b2638 --- /dev/null +++ b/sql/injection/testdata/src/s/main.go @@ -0,0 +1,16 @@ +package main + +import ( + "net/http" + + "github.com/Masterminds/squirrel" +) + +func handler(w http.ResponseWriter, r *http.Request) { + squirrel.Expr(r.URL.Query().Get("q")) // want "potential sql injection" +} + +func main() { + http.HandleFunc("/", handler) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/t/main.go b/sql/injection/testdata/src/t/main.go new file mode 100644 index 0000000..a8de289 --- /dev/null +++ b/sql/injection/testdata/src/t/main.go @@ -0,0 +1,24 @@ +package main + +import ( + "net/http" + + "github.com/go-gorm/gorm" +) + +type User struct { + gorm.Model + Name string +} + +func handle(db *gorm.DB, q string) { + db.Where(q).Find(&User{}) // want "potential sql injection" +} + +func main() { + var db *gorm.DB + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + handle(db, r.URL.Query().Get("q")) + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/u/main.go b/sql/injection/testdata/src/u/main.go new file mode 100644 index 0000000..67034a6 --- /dev/null +++ b/sql/injection/testdata/src/u/main.go @@ -0,0 +1,16 @@ +package main + +import ( + "database/sql" + "net/http" + + _ "github.com/mattn/go-sqlite3" +) + +func main() { + db, _ := sql.Open("sqlite3", ":memory:") + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + db.Query(r.URL.Query().Get("q")) // want "potential sql injection" + }) + http.ListenAndServe(":8080", nil) +} diff --git a/sql/injection/testdata/src/xorm.io/xorm/mock.go b/sql/injection/testdata/src/xorm.io/xorm/mock.go new file mode 100644 index 0000000..f23f822 --- /dev/null +++ b/sql/injection/testdata/src/xorm.io/xorm/mock.go @@ -0,0 +1,41 @@ +package xorm + +type Engine struct{} + +type Session struct{} + +func (e *Engine) Query(string, ...interface{}) {} +func (e *Engine) Exec(string, ...interface{}) {} +func (e *Engine) QueryString(string, ...interface{}) {} +func (e *Engine) QueryInterface(string, ...interface{}) {} +func (e *Engine) SQL(string, ...interface{}) *Session { return nil } +func (e *Engine) Where(string, ...interface{}) *Session { return nil } +func (e *Engine) And(string, ...interface{}) *Session { return nil } +func (e *Engine) Or(string, ...interface{}) *Session { return nil } +func (e *Engine) Alias(string) *Session { return nil } +func (e *Engine) NotIn(string, ...interface{}) *Session { return nil } +func (e *Engine) In(string, ...interface{}) *Session { return nil } +func (e *Engine) Select(string) *Session { return nil } +func (e *Engine) SetExpr(string, string) *Session { return nil } +func (e *Engine) OrderBy(string) *Session { return nil } +func (e *Engine) Having(string) *Session { return nil } +func (e *Engine) GroupBy(string) *Session { return nil } +func (e *Engine) Join(string, interface{}, string, ...interface{}) *Session { return nil } + +func (s *Session) Query(string, ...interface{}) {} +func (s *Session) Exec(string, ...interface{}) {} +func (s *Session) QueryString(string, ...interface{}) {} +func (s *Session) QueryInterface(string, ...interface{}) {} +func (s *Session) SQL(string, ...interface{}) *Session { return nil } +func (s *Session) Where(string, ...interface{}) *Session { return nil } +func (s *Session) And(string, ...interface{}) *Session { return nil } +func (s *Session) Or(string, ...interface{}) *Session { return nil } +func (s *Session) Alias(string) *Session { return nil } +func (s *Session) NotIn(string, ...interface{}) *Session { return nil } +func (s *Session) In(string, ...interface{}) *Session { return nil } +func (s *Session) Select(string) *Session { return nil } +func (s *Session) SetExpr(string, string) *Session { return nil } +func (s *Session) OrderBy(string) *Session { return nil } +func (s *Session) Having(string) *Session { return nil } +func (s *Session) GroupBy(string) *Session { return nil } +func (s *Session) Join(string, interface{}, string, ...interface{}) *Session { return nil } From b0ddb53f016caf1140288f1f35f445b43c8d7b89 Mon Sep 17 00:00:00 2001 From: Kent Gruber Date: Fri, 4 Jul 2025 12:02:12 -0400 Subject: [PATCH 2/5] Address PR feedback --- sql/injection/injection.go | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/sql/injection/injection.go b/sql/injection/injection.go index 657f53f..c8e07c9 100644 --- a/sql/injection/injection.go +++ b/sql/injection/injection.go @@ -231,26 +231,28 @@ func imports(pass *analysis.Pass, pkgs ...string) bool { return imported } +var supportedSQLPackages = []string{ + "database/sql", + "github.com/mattn/go-sqlite3", + "github.com/jinzhu/gorm", + "gorm.io/gorm", + "github.com/go-gorm/gorm", + "github.com/jmoiron/sqlx", + "xorm.io/xorm", + "github.com/go-xorm/xorm", + "github.com/go-pg/pg", + "github.com/rqlite/gorqlite", + "github.com/raindog308/gorqlite", + "github.com/Masterminds/squirrel", + "gopkg.in/Masterminds/squirrel.v1", + "github.com/lann/squirrel", +} + func run(pass *analysis.Pass) (interface{}, error) { // Require at least one supported SQL package to be imported before // running the analysis. This avoids wasting time analyzing programs // that do not use SQL. - if !imports(pass, - "database/sql", - "github.com/mattn/go-sqlite3", - "github.com/jinzhu/gorm", - "gorm.io/gorm", - "github.com/go-gorm/gorm", - "github.com/jmoiron/sqlx", - "xorm.io/xorm", - "github.com/go-xorm/xorm", - "github.com/go-pg/pg", - "github.com/rqlite/gorqlite", - "github.com/raindog308/gorqlite", - "github.com/Masterminds/squirrel", - "gopkg.in/Masterminds/squirrel.v1", - "github.com/lann/squirrel", - ) { + if !imports(pass, supportedSQLPackages...) { return nil, nil } @@ -328,7 +330,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } // Skip the context argument, if using a *Context query variant. - if strings.HasPrefix(queryEdge.Site.Value().Call.Value.String(), "Context") { + if strings.HasSuffix(queryEdge.Site.Value().Call.Value.String(), "Context") { if len(queryArgs) < 2 { continue } From f2ae2e092ba0fd67538516ac7f47eb920643c8f2 Mon Sep 17 00:00:00 2001 From: Kent Gruber Date: Fri, 4 Jul 2025 12:19:21 -0400 Subject: [PATCH 3/5] extend SQL package detection --- sql/injection/injection.go | 24 ++++++++++++------- sql/injection/injection_test.go | 4 ++++ .../sqreen/go-dvwa/vulnerable/sql.go | 23 ++++++++++++++++++ sql/injection/testdata/src/v/main.go | 15 ++++++++++++ 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 sql/injection/testdata/src/github.com/sqreen/go-dvwa/vulnerable/sql.go create mode 100644 sql/injection/testdata/src/v/main.go diff --git a/sql/injection/injection.go b/sql/injection/injection.go index c8e07c9..4f20c01 100644 --- a/sql/injection/injection.go +++ b/sql/injection/injection.go @@ -2,6 +2,7 @@ package injection import ( "fmt" + "go/types" "strings" "github.com/picatz/taint" @@ -216,19 +217,26 @@ var Analyzer = &analysis.Analyzer{ // imports returns true if the package imports any of the given packages. func imports(pass *analysis.Pass, pkgs ...string) bool { - var imported bool - for _, imp := range pass.Pkg.Imports() { + visited := make(map[*types.Package]bool) + var walk func(*types.Package) bool + walk = func(p *types.Package) bool { + if visited[p] { + return false + } + visited[p] = true for _, pkg := range pkgs { - if strings.HasSuffix(imp.Path(), pkg) { - imported = true - break + if strings.HasSuffix(p.Path(), pkg) { + return true } } - if imported { - break + for _, imp := range p.Imports() { + if walk(imp) { + return true + } } + return false } - return imported + return walk(pass.Pkg) } var supportedSQLPackages = []string{ diff --git a/sql/injection/injection_test.go b/sql/injection/injection_test.go index b61a178..8fab3b0 100644 --- a/sql/injection/injection_test.go +++ b/sql/injection/injection_test.go @@ -97,3 +97,7 @@ func TestT(t *testing.T) { func TestU(t *testing.T) { analysistest.Run(t, testdata, Analyzer, "u") } + +func TestV(t *testing.T) { + analysistest.Run(t, testdata, Analyzer, "v") +} diff --git a/sql/injection/testdata/src/github.com/sqreen/go-dvwa/vulnerable/sql.go b/sql/injection/testdata/src/github.com/sqreen/go-dvwa/vulnerable/sql.go new file mode 100644 index 0000000..80d5c79 --- /dev/null +++ b/sql/injection/testdata/src/github.com/sqreen/go-dvwa/vulnerable/sql.go @@ -0,0 +1,23 @@ +package vulnerable + +import ( + "context" + "database/sql" +) + +type Product struct { + ID int + Name string + Category string +} + +func PrepareSQLDB() (*sql.DB, error) { return nil, nil } + +func GetProducts(ctx context.Context, db *sql.DB, category string) ([]Product, error) { + rows, err := db.QueryContext(ctx, "SELECT * FROM product WHERE category='"+category+"'") // want "potential sql injection" + if err != nil { + return nil, err + } + _ = rows + return nil, nil +} diff --git a/sql/injection/testdata/src/v/main.go b/sql/injection/testdata/src/v/main.go new file mode 100644 index 0000000..037cac6 --- /dev/null +++ b/sql/injection/testdata/src/v/main.go @@ -0,0 +1,15 @@ +package main + +import ( + "net/http" + + "github.com/sqreen/go-dvwa/vulnerable" +) + +func main() { + db, _ := vulnerable.PrepareSQLDB() + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + vulnerable.GetProducts(r.Context(), db, r.FormValue("category")) + }) + http.ListenAndServe(":8080", nil) +} From 03c62ff86b252e19423753d4ac689a4e542548f2 Mon Sep 17 00:00:00 2001 From: Kent Gruber Date: Fri, 4 Jul 2025 12:31:33 -0400 Subject: [PATCH 4/5] Recognize FormValue as source --- sql/injection/injection.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/injection/injection.go b/sql/injection/injection.go index 4f20c01..5fa7713 100644 --- a/sql/injection/injection.go +++ b/sql/injection/injection.go @@ -25,8 +25,8 @@ var userControlledValues = taint.NewSources( // "(*net/url.Userinfo).Passworde", // "(*net/url.Userinfo).String", // "(*net/http.Request).FormFile", - // "(*net/http.Request).FormValue", - // "(*net/http.Request).PostFormValue", + "(*net/http.Request).FormValue", + "(*net/http.Request).PostFormValue", // "(*net/http.Request).Referer", // "(*net/http.Request).UserAgent", // "(*net/http.Request).GetBody", From c969ebb7f8cdd883c9aaaede8150a0f57b892b8d Mon Sep 17 00:00:00 2001 From: Kent 'picat' Gruber Date: Sun, 6 Jul 2025 15:40:42 -0400 Subject: [PATCH 5/5] Remove redundant sources --- sql/injection/injection.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/injection/injection.go b/sql/injection/injection.go index 5fa7713..a055344 100644 --- a/sql/injection/injection.go +++ b/sql/injection/injection.go @@ -16,17 +16,21 @@ import ( // userControlledValues are the sources of user controlled values that // can be tained and end up in a SQL query. var userControlledValues = taint.NewSources( - // Function (and method) calls + // Function (and method) calls that are user controlled + // over the netork. These are all taken into account as + // part of *net/http.Request, but are listed here for + // demonstration purposes. + // // "(net/url.Values).Get", // "(*net/url.URL).Query", // "(*net/url.URL).Redacted", // "(*net/url.URL).EscapedFragment", // "(*net/url.Userinfo).Username", - // "(*net/url.Userinfo).Passworde", + // "(*net/url.Userinfo).Password", // "(*net/url.Userinfo).String", // "(*net/http.Request).FormFile", - "(*net/http.Request).FormValue", - "(*net/http.Request).PostFormValue", + // "(*net/http.Request).FormValue", + // "(*net/http.Request).PostFormValue", // "(*net/http.Request).Referer", // "(*net/http.Request).UserAgent", // "(*net/http.Request).GetBody",