proposal: database/sql: provide google/go-safeweb/safesql like api

[empijei]

Currently the database/sql package does very little to prevent accidental SQL injection.

Both of these calls are valid and accepted:

db.QueryRow("SELECT id from db where name = ?", name) // Safe
db.QueryRow("SELECT id from db where name = '"+name+"'") // SQL injection vulnerable

There are ways to make sure that only constants or trusted strings end up being used as query sources, like the safesql package does.

Consider that the safesql package is a very tiny wrapper around the standard package, most code looks like this:

func (c Conn) ExecContext(ctx context.Context, query TrustedSQLString, args ...interface{}) (Result, error) {
    return c.c.ExecContext(ctx, query.s, args...)
}

And the only real addition is the trusted string manipulation and construction library provided by the fifty lines of code in safesql.go.

Maintaining this package is trivial as it only changes when the sql API package changes, and it contains very little logic.

The advantage is that this makes it significantly harder for programmers to accidentally pass user input as query source, without almost any hinderance on code writing, and migration is trivial (see more in the doc).

Code like the following is trivial to migrate from sql to safesql: db.Query("SELECT ...", args...) The only change required would be to promote the string literal to a trusted string: db.Query(safesql.New("SELECT ..."), args...)

This approach is the one used and suggested by Google (the safesql package is itself owned by Google) and it has been talked about for a while now (2016 talk by Christoph Kern). It's a very battle tested and effective way to prevent code injection.

The standard library has packages like html/template that already follow similar approaches, so this is not new to the Go stdlib.

Sadly, this is not known and most people end up using the barebone, unprotected SQL package.

My proposal would be to provide a v2 for database/sql, a database/safesql or even just a separate package outside of the stdlib that the doc for database/sql points to.

I think having users default to the less safe option is a suboptimal situation, we should at least provide a warning of the issue and point users to a solution.

[empijei]

/cc @FiloSottile @kele @bsiegert @mikispag

[seankhliao]

see also #22697 for general sql v2 wishlist

[rsc]

The standard library has packages like html/template that already follow similar approaches

I'm not sure what "similar" means here in the context of html/template. I don't believe it has APIs that insist on TrustedFoo.

[empijei]

What I meant is that html/template doesn't just take string. Strings are by default escaped and treated as untrusted, for a string to be rendered verbatim in an action it requires to be promoted to HTML, for example.

But it does have some issues with accepting bare strings as template sources, in fact potentially accepting userg-generated content as template source, but that's a much more complex issue that would require a big change to be addressed (e.g. going the safehtml way).

Consider that an acceptable middle-ground would be to use a type alias and rely on vet to make sure that the type is only ever constructed from string manipulation of the safe type or from constants. This would not require an API change but it would surface issues when people vet their code (which, hopefully, happens early in the dev cycle).