Query builders hide SQL vulnerabilities

[ubombi]

Summary

Using query builders along with manual string formatting or concatenation isn't detected.

Steps to reproduce the behavior

sb = squirrel.StatementBuilder.Select("1")                                                                                                                                                                                   
sb = sb.Where(
    fmt.Sprintf(
        "somefield IN ( SELECT foo.bar FROM foo JOIN bar ON (bar.id = foo.bar_id) WHERE foo.value IN ('%v') )", 
        strings.Join(inputArray, "','"),
    ),
)

/* q, args, _ := sb.ToSql()
err = r.db.GetContext(ctx, &res, q, args...)
...
*/

go v1.21.5 | Linux amd64 |

Expected behavior

G201

Actual behavior

Nothing found

Possible fix

I'm not familiar with the codebase and just brute-forced what I needed, and I'm not sure if it's a proper way to do it.
If you would find approach this appropriate, I can make a proper PR with all sqlCallIdents

diff --git a/rules/sql.go b/rules/sql.go
index 61222bf..7459e2f 100644
--- a/rules/sql.go
+++ b/rules/sql.go
@@ -52,6 +52,10 @@ var sqlCallIdents = map[string]map[string]int{
                "Prepare":         0,
                "PrepareContext":  1,
        },
+       "github.com/Masterminds/squirrel.SelectBuilder": {
+               "Where": 0,
+               // and all other methods
+       },
 }
 
 // findQueryArg locates the argument taking raw SQL
@@ -268,6 +272,13 @@ func (s *sqlStrFormat) checkQuery(call *ast.CallExpr, ctx *gosec.Context) (*issu
                return nil, err
        }
 
+       if call, ok := query.(*ast.CallExpr); ok && call.Fun != nil {
+               issue := s.checkFormatting(call, ctx)
+               if issue != nil {
+                       return issue, err
+               }
+       }
+
        if ident, ok := query.(*ast.Ident); ok && ident.Obj != nil {
                decl := ident.Obj.Decl
                if assign, ok := decl.(*ast.AssignStmt); ok {

Cons

This opens a path to the support of 3rd party libraries, and there are a lot of them

[ccojocar]

Thanks for opening this issue. I am not sure about the fix. I would avoid to hardcode 3rd party libraries into the rule. Maybe something configurable would be more appropriate.

[ubombi]

I don't like that part too. On the other hand, not many people (who use ORMs / QBs) would configure gosec, using it as part of golangci-lint.