Actually, what I really wanted to blog about before getting carried away with irony yesterday was an old idea on how to force my developers to use placeholders exclusively for SQL queries in applications. As should be apparent from yesterdays blog entry, I am strongly in favour of using placeholders for interpolating values into SQL queries, due to the great reduction in potential bugs (including, but not limited to, SQL injections).
Basically, wrap the database API so that all database access passes through the wrapper. This can usually be achieved, for example by subclassing DBI (for Perl) and returning such subclasses from the application connection pool, or other similar methods. Probably many large web applications already have such wrappers or use APIs that can be patched or extended appropriately.
Now add code that basically bombs out with a big error message if any SQL query contains a quote character. Something like “Always use placeholders for interpolating values into SQL queries! If in disagreement, go see your development lead for your regular spanking!” or words to the same effect.
Sometimes, the wrapper may sit below some code in the database API that emulates placeholders (for example, DBD::mysql used to emulate placeholders in the client using mysql_real_quote_string() or equivalent, since real server-side placeholders are only available with the newer version of the MySQL protocol for prepared statements). But even in this case, the wrapper can still force the use of placeholders by exploiting the fact that MySQL supports both single (‘)and double (“) quotes. Basically, the wrapper would set some private global variable at random to either a single or a double quote, make placeholder emulation use one, and bomb out if the other is detected in query strings. Then any developer trying to sneak manual quoting into the application would quickly be caught, and subsequently taught.
The technique is not perfect. It does not catch completely unquoted number interpolation (shudder). It will also be somewhat of an annoyance to have to specify all string constants as placeholders (there is nothing wrong with "SELECT value FROM t WHERE id = ? AND color = 'red'"). In the end, I never got to implement it, also because my team was small enough and clue-full enough that normal face-to-face talk was sufficient to make placeholders be used throughout.
But if I ever find myself as lead or architect for a web application team, I will be sorely tempted to implement it, as an educational means for the developers and just to see what reactions it will cause.
False security
SQL injection does not come from quoted strings, it comes from interpolating invalid content into a query. Your wrapper check cannot detect all cases of interpolated content, because not all cases involve quoted strings. The example below has replaced its quoted string with a placeholder, but it still contains a SQL injection attack!
UPDATE accounts SET PASSWORD = ? WHERE account_id = 123 OR TRUE;
You can’t automate protection against all cases of SQL injection. If you try, you just create a false sense of security — as though by having automated checks in place you are somehow immune. See http://www.greensql.net/.
Instead, you should conduct effective code reviews to look for cases of SQL injection and other bugs.
Re: False security
This is not meant to automatically protect against SQL injection. I very much agree that there is no good way to automatically protect against SQL injection, and absolutely hate attempts in that direction like the “magic quoting” of PHP (those make matters even _worse_ in fact by complicating the semantics of the language even further).
On the contrary, the purpose of this technique is to intentionally break otherwise correct code that does not use placeholders, in order to educate developers. If you cannot pass a query string containing a quote character, it becomes quite difficult to interpolate eg. a username to perform a lookup! So this will force the developer to think about the issue, ask why this is so, and be educated.
Escaping and interpolating values into SQL query strings is just wrong when placeholders are available. SQL injection is just one example of the potential problems it can create.
Re: False security
Actually, there is a better – more robust and much less annoying – technique available in perl, simply consider all user input tainted. When you interpolate it into a query, it automatically becomes tainted too. Don’t allow your wrapper to execute tainted queries. But passing a tainted data as a placeholder value should be allowed. Note that it doesn’t prevent developer from using hard-coded constants (like in your AND color = ‘red’ example), so it’s practically invisible in all normal cases. DBI should do it out of the box 🙂