go: database/sql: missing escape functions

quoting query text is part of the database interface.

I would argue to the contrary. Escaping primitives are needed for working securely with databases and they are inherently dependent on the SQL-server behind the connection. A package using database.sql might not even know what server it is talking to and how to properly quote and escape for that server, if it just gets passed a reference.

As is, database.sql can only be used for queries known ahead of time and under the constraint, that the only dynamic entities in the query are value and never columns (think ORDER BY <user input>) or tables

I didn’t say “me too”, actually. I referenced a good implementation. Check out DBI and quote_identifier / quote_literal functions. Just like database/sql, DBI leaves platform-specific implementation up to the drivers.

I’m new to Go so my apologies if this is extraordinarily obvious to others, but I have managed to use Sprintf to create a string for the query containing a dynamic table name for Sqlite3. … This has allowed me to query my Sqlite3 database using a dynamic table name which comes from elsewhere in my program (never as user input). Hope this helps someone though I understand this discussion is more tailored around larger DB systems. Who knows, it might work with them too!

We now have very short steps for someone to take to a disaster:

1. someone tries using placeholders for column names and finds that this fails
2. this person asks around, finds out that these need special quoting, and looks for this feature in database/sql–and doesn’t find it
3. this person searches the internet, finds this issue, scrolls down, aha! Here is helpful code to use.
4. this person doesn’t notice the bold never or thinks (wrongly) that it’s fine
5. this person has a trivial SQL injection vulnerability! yay!

This person isn’t me, because I’m translating code using Perl’s DBI and DBI did this right decades ago. We’ve have decades of trivial SQL vulnerabilities due to string construction of queries from untrusted input, and when placeholders are finally pretty common, people are still pooh-poohing other necessary measures with the exact same arguments used to say that placeholders weren’t necessary? This is disgraceful.

@caseyallenshobe, see https://golang.org/wiki/NoMeToo

If you have a proposal, please propose.

I’m honestly kinda surprised that after 6 1/2 years there doesn’t seem to be a standard way of escaping a user-input string in sql using go.

So proper quoting is impossible, since neither database.sql nor database.sql.driver export quoting functions and when I am sitting behind those apis my best bet is type-switching the driver to find out what I am talking to and worst case implementing that my self? That does not look solid to me.

For: github.com/go-sql-driver/mysql

I couldn’t stand to keep mexing mixing quotes with + concatenation with fmt.Sprintf so I tried to create a standard.

I know this is far from perfect but I will share what I did and fixed for me: <snip>

Your solution does not escape input strings, its a workaround to being unable to use backticks in a multi-line literal.

This issue was opened because string replace isn’t a proper solution. It makes the application vulnerable to SQL injection.

What we asking for is to add func (*sql.Conn) Escape(string) (string, err) which calls func (*driver.Driver) Escape(*sql.Conn, string) (string, err) which then does the correct driver-specific escaping by calling mysql_real_escape_string or similar.

Yes, it’s possible to roll your own solution by following the docs but the whole point of the database/sql package is that users don’t have to deal with driver-specific nonsense.

It’s only been 7 years, there is no rush.

A temporary solution for strings from the Internet:

func escape(source string) string {
	var j int = 0
	if len(source) == 0 {
		return ""
	}
	tempStr := source[:]
	desc := make([]byte, len(tempStr)*2)
	for i := 0; i < len(tempStr); i++ {
		flag := false
		var escape byte
		switch tempStr[i] {
		case '\r':
			flag = true
			escape = '\r'
			break
		case '\n':
			flag = true
			escape = '\n'
			break
		case '\\':
			flag = true
			escape = '\\'
			break
		case '\'':
			flag = true
			escape = '\''
			break
		case '"':
			flag = true
			escape = '"'
			break
		case '\032':
			flag = true
			escape = 'Z'
			break
		default:
		}
		if flag {
			desc[j] = '\\'
			desc[j+1] = escape
			j = j + 2
		} else {
			desc[j] = tempStr[i]
			j = j + 1
		}
	}
	return string(desc[0:j])
}

@kardianos For example, INSERT INTO mytable (a,b,c) VALUES (a1,b1,c1), (a2,b2,c2), (a3,b3,c3), … (an,bn,cn);

That’s more helpful. For what it’s worth, I lived and breathed Perl and Perl’s DBI for a decade+, and that’s what inspired my writing the database/sql package.

I’ll leave this to @kardianos, who’s taken over maintenance of the package.

Placeholders are not abstracted. database/sql itself doesn’t care at all, it just passes the query to a driver. And most drivers I know of don’t abstract it away, either, requiring you to use $1 for PostgreSQL and ? for SQLite.

Similarly, quoting arguments would depend on the specific DBMS.

@kardianos

Maybe I’m being naive, but I would expect that quoting would have to be the responsibility of the driver have to be at the connection level to get it right. (MySQL being the obvious case in point.)

It would then be up to the sql package to provide an interface to obtain the quoter from the connection. An error return value allows for compatibility with drivers that have not implemented a quoter. Something like:

// Quoter provides a driver-appropriate utility for quoting values and identifiers.
//
// If the driver does not provide a Quoter, then a nil value is returned with a 
// non-nil error.
func (*sql.Conn) Quoter() (Quoter, error) { ... }

If I’m in a project and I feel good with the assumption that my driver of choice provides a Quoter, then I can just ignore the error case. If I want maximum abstraction, I write a few extra lines to handle that case.

I’m new to Go so my apologies if this is extraordinarily obvious to others, but I have managed to use Sprintf to create a string for the query containing a dynamic table name for Sqlite3.

import (
        _ "github.com/mattn/go-sqlite3"
    )

func main() {
            
	var result string
	tableName := "testTable"
	value := "10"
	dynamicQuery := fmt.Sprintf("select myField from '%s' where myValue = '%s'", tableName, value)

        database, _ := sql.Open("sqlite3", "/path/to/my/Sqlite3.db")
	rows, err := database.Query(dynamicQuery)
	defer rows.Close()
	if err != nil {
            fmt.Println("Error running db Query.")
	    fmt.Println(err)
	} else {
            for rows.Next() {
                err := rows.Scan(&result)
                if err != nil {
                    fmt.Println("Error scanning row")
                }
	        fmt.Printf("Table result is %v", result)
	    }
	}
}

This has allowed me to query my Sqlite3 database using a dynamic table name which comes from elsewhere in my program (never as user input). Hope this helps someone though I understand this discussion is more tailored around larger DB systems. Who knows, it might work with them too!

MySQL has configuration options for quoting, so the quoting is also dependent on the connection.

As a user I would expect the function to be on the sql.DB object, since I probably already have a reference to it when I am constructing queries and sql.DB already has all the information to set up the quoting. Thats also where you find them in similar packages for other languages like Perl/DB, PHP/PDO, PHP/Mysqli.

There is also the issue, that quoting rules are database or even connection dependent, so if the drivers do not provide quoting functions, there is no way to write database agnostic code. I fail to see why the maintainers chose not to require drives to provide such functions. Database vendors do provide quoting functions in their C libraries, for good reasons.

Ignoring multi-row statements for now (off topic), what’s the current state of getting a quoter into the stdlib? Is this one of those issues which might be marked as help-wanted? If I have some free time on my hands (and some experience with mentioned prior art like PHP PDO and mysqli), what are the best steps I could make to help move this along (write code, put together a proposal,…?)

For: github.com/go-sql-driver/mysql

I couldn’t stand to keep mexing quotes with + concatenation with fmt.Sprintf so I tried to create a standard.

I know this is far from perfect but I will share what I did and fixed for me:

Code

package repository

import (
	"fmt"
	"reflect"
	"strings"
)

type Repository struct{}

type BuildQueryParams struct {
	Placeholders []BuildQueryPlaceholder
}

type BuildQueryPlaceholder struct {
	Key    string
	Params []interface{}
}

// NewRepository get a new instance
func NewRepository() *Repository {
	return &Repository{}
}

func (r *Repository) BuildQuery(rawSql string, buildQueryParams BuildQueryParams) (string, []interface{}) {
	finalQuery := strings.ReplaceAll(rawSql, "%q", "`")
	finalParams := []any{}
	for _, placeholder := range buildQueryParams.Placeholders {
		pMask, params := r.getPlaceholders(placeholder.Params...)
		finalParams = append(finalParams, params...)
		finalQuery = strings.ReplaceAll(finalQuery, fmt.Sprintf(":%s", placeholder.Key), pMask)
	}
	return finalQuery, finalParams
}

func (r *Repository) SpreadSlice(slice any) []interface{} {
	s := reflect.ValueOf(slice)
	if s.Kind() != reflect.Slice {
		panic("InterfaceSlice() given a non-slice type")
	}

	// Keep the distinction between nil and empty slice input
	if s.IsNil() {
		return nil
	}

	ret := make([]interface{}, s.Len())

	for i := 0; i < s.Len(); i++ {
		ret[i] = s.Index(i).Interface()
	}
	return ret
}

func (r *Repository) getPlaceholders(values ...interface{}) (placeholders string, parameters []interface{}) {
	n := len(values)
	p := make([]string, n)
	parameters = make([]interface{}, n)
	for i := 0; i < n; i++ {
		p[i] = "?"
		parameters[i] = values[i]
	}
	placeholders = strings.Join(p, ",")
	return placeholders, parameters
}

Usage

sql, params := r.repository.BuildQuery(`
SELECT 	
        ch.%qname%q,
	ch.client_username
FROM %qcharacter%q ch
INNER JOIN %qclient%q cl ON ch.client_username = cl.username
WHERE ch.%qname%q IN (:p1)
AND cl.client_status_id = :p2
AND ch.character_status_id = 1;`, BuildQueryParams{
Placeholders: []BuildQueryPlaceholder{
	{
		Key: "p1",
		Params: r.repository.BindSlice([]any{
			"character1", "character2", "character3",
		}),
	},
	{
		Key: "p2",
		Params: r.repository.BindSlice([]any{
			1,
		}),
	},
},
})

fmt.Println("Final SQL", sql)
fmt.Println("Final Params", params)

Output

Final SQL 
SELECT 	
               ch.`name`,
               ch.client_username
FROM `character` ch
INNER JOIN `client` cl ON ch.client_username = cl.username
WHERE ch.`name` IN (?,?,?)
AND cl.client_status_id = ?
AND ch.character_status_id = 1;

Final Params [character1 character2 character3 1]

Query it

rows, err := connection.QueryContext(executionCtx, sql, params...)

The final sql will be replaced with “sql quotes” for each %q and the placeholders automatically added for each mapped key that starts with semi colon (😃.

All placerholder parameters will be combined in sequence to be used for the prepared statement bind.

BindSlice function is used to generically pass any kind of slice to be appended to final parameters.

That’s is!

@nefthy Good point about MySQL especially. I’ve updated the calls to be network friendly.

You can get the driver from sql.DB.Driver() so while it isn’t a direct method from it in this shim, you can use in in a similar way right now.

Here’s what I’d like to see:

1. evolve and verify the API that is in sqlexp,
2. get a few drivers to implement it (they usually appreciate PRs),
3. then if it makes sense (which I think it would at that point), bring it into the std lib.

Perhaps it would be good to combine the escaper functions with the database name function. We’d probably want to make it easy to expand it in the future as well, which would mean changing it into a struct with methods.