[ovex]

Interesting. I would still insist on not doing it that way, especially when writing a library for universal use. First of all, string replacement on structured input is an immediate red flag. Second, even if you get handling the structured input right by parsing the statement into an AST and replacing the leaves that are placeholders with the escaped strings, there are still potential vulnerabilities. For example, mysql_real_escape_string is still vulnerable when being used with some exotic character sets [1].

I don't know the internals of the (binary) protocol used for communication with the MySQL server though. Couldn't one just save the extra round-trip with length-prefixed strings by sending the query together with the parameters in a single message?

[1]: http://www.gosecure.it/blog/art/483/sec/mysql_escape_string-...

[bgrainger]

> Couldn't one just save the extra round-trip with length-prefixed strings by sending the query together with the parameters in a single message?

No, this wouldn't work, because you have to send COM_STMT_PREPARE (https://dev.mysql.com/doc/internals/en/com-stmt-prepare.html) first, which takes the SQL and returns a "statement ID". Then you can send COM_STMT_EXECUTE (https://dev.mysql.com/doc/internals/en/com-stmt-execute.html) which contains the statement ID and the parameters. Finally, you would ideally send COM_STMT_CLOSE (https://dev.mysql.com/doc/internals/en/com-stmt-close.html) to free the server-side resources for the prepared statement, although this could be "pipelined" with the EXECUTE packet.

[evanelias]

> mysql_real_escape_string is still vulnerable when being used with some exotic character sets

Indeed -- mysql_real_escape_string "mostly" fixes this problem by requiring a db connection as one of its args. Since the driver is usually aware of the connection state, mysql_real_escape_string can check to see if one of those exotic charsets is in-use. The issue is that there are multiple ways to change the connection charset, some of which the driver is aware of (e.g. in PHP mysqli set_charset) but some it is not (running textual statements like SET NAMES or SET CHARACTER SET).

However, generally an attacker won't have the ability to set an arbitrary exotic character set for the connection anyway... unless they already have some other sql injection mechanism, in which case it's a moot point :)

Driver documentation also typically mentions this problem. For example, here's the doc for doing client-side param interpolation in the most popular MySQL driver for Golang: https://github.com/go-sql-driver/mysql#interpolateparams (see warning in italics)

It also explicitly detects if your initial connection settings attempt to use one of those charsets along with param interpolation, and throws an error if so: https://github.com/go-sql-driver/mysql/blob/21f789cd/dsn.go#...

> Couldn't one just save the extra round-trip with length-prefixed strings by sending the query together with the parameters in a single message?

AFAIK, no, not with the traditional MySQL binary protocol. The newer "X protocol" introduced in MySQL 5.7 does allow this, but it is not widely implemented in drivers.

[nicoburns]

I actually think it’s fine to do string sanitising here. The string formats for the databases are well specified, and writing a function to that spec shouldn’t be difficult. I believe there’s no need to parse to AST for postgres as you can just wrap the string in quotes, replace any existing quotes in the string with two quotes and you’re done. If other databases have funkier string formats then you may have a point.

at least that’s true for Postgres, I can’t speak for MySQL.