Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false negative for SQL injection when using DB.QueryRow.Scan() #759

Merged
merged 3 commits into from Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions rules/sql.go
Expand Up @@ -261,6 +261,21 @@ func (s *sqlStrFormat) Match(n ast.Node, ctx *gosec.Context) (*gosec.Issue, erro
switch stmt := n.(type) {
case *ast.AssignStmt:
for _, expr := range stmt.Rhs {
// when expr is CallExpr and Call.Fun.X is CallExpr ,check Call.Fun.X
kaiili marked this conversation as resolved.
Show resolved Hide resolved
if Call, ok := expr.(*ast.CallExpr); ok {
kaiili marked this conversation as resolved.
Show resolved Hide resolved
Selector, ok := Call.Fun.(*ast.SelectorExpr)
kaiili marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
continue
}
sqlQueryCall, ok := Selector.X.(*ast.CallExpr)
// checkQuery when success get sqlQuery CallExpr and Selector contains Callexpr
kaiili marked this conversation as resolved.
Show resolved Hide resolved
if ok && s.ContainsCallExpr(sqlQueryCall, ctx) != nil {
issue, err := s.checkQuery(sqlQueryCall, ctx)
if err == nil && issue != nil {
return issue, err
}
}
}
if sqlQueryCall, ok := expr.(*ast.CallExpr); ok && s.ContainsCallExpr(expr, ctx) != nil {
return s.checkQuery(sqlQueryCall, ctx)
}
Expand Down
69 changes: 69 additions & 0 deletions testutils/source.go
Expand Up @@ -1189,6 +1189,75 @@ func main(){
panic(err)
}
defer rows.Close()
<<<<<<< HEAD
kaiili marked this conversation as resolved.
Show resolved Hide resolved
}`}, 1, gosec.NewConfig()}, {[]string{`
// Format string with \n\r
package main

import (
"database/sql"
"fmt"
"os"
)

func main(){
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
q := fmt.Sprintf("SELECT * FROM foo where\nname = '%s'", os.Args[1])
rows, err := db.Query(q)
if err != nil {
panic(err)
}
defer rows.Close()
}`}, 1, gosec.NewConfig()}, {[]string{`
// SQLI by db.Query(some).Scan(&other)
package main

import (
"database/sql"
"fmt"
"os"
)

func main() {
var name string
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
q := fmt.Sprintf("SELECT name FROM users where id = '%s'", os.Args[1])
row := db.QueryRow(q)
err = row.Scan(&name)
if err != nil {
panic(err)
}
defer db.Close()
}`}, 1, gosec.NewConfig()}, {[]string{`
// SQLI by db.Query(some).Scan(&other)
package main

import (
"database/sql"
"fmt"
"os"
)

func main() {
var name string
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
panic(err)
}
q := fmt.Sprintf("SELECT name FROM users where id = '%s'", os.Args[1])
err = db.QueryRow(q).Scan(&name)
if err != nil {
panic(err)
}
defer db.Close()
=======
kaiili marked this conversation as resolved.
Show resolved Hide resolved
>>>>>>> 58058af0c8275f11249a71f18fc548bbd7b97ccc
kaiili marked this conversation as resolved.
Show resolved Hide resolved
}`}, 1, gosec.NewConfig()},
}

Expand Down