Safesql: args are not spread out when calling wrapped database/sql functions

[mattiasgrenfeldt]

Hello friends! I tried using safesql and it didn't work out that well.

database/sql is not wrapped properly in safesql/sqlwrap.go. The args are not spread out in the call to the underlying functions. Here is Exec as an example:

// Exec is a tiny wrapper for https://pkg.go.dev/sql#DB.Exec
func (db DB) Exec(query TrustedSQLString, args ...interface{}) (Result, error) {
	return db.db.Exec(query.s, args) // <-- Should be args...
}

This seems to be the case for all functions taking an args ...interface{} in sqlwrap.go.
Here is a small POC:

package main

import (
	"fmt"
	"log"

	"github.com/google/go-safeweb/safesql"
	_ "github.com/jackc/pgx/v4/stdlib"
)

func main() {
	db, err := safesql.Open("pgx", "postgres://postgres:password@localhost:5432/")
	if err != nil {
		log.Fatal(err)
	}
	if _, err := db.Exec(safesql.New("CREATE TABLE blah (id int)")); err != nil {
		log.Fatalf("db.Exec got error: %v", err)
	}
	fmt.Println("It worked")
}

Running this gives:

2020/12/02 23:32:45 db.Exec got error: expected 0 arguments, got 1
exit status 1

But if i run the same code using database/sql it works.

[kele]

Huh, this is an interesting bug around ... interface{}! I wonder if a tool like semgrep could help with that (tagged dgryski at twitter who AFAIU owns Go's semgrep): https://twitter.com/kele_codes/status/1335995071557218305

[empijei]

Thanks for reporting this. This is now fixed.