89

Are prepared statements actually 100% safe against SQL injection, assuming all user-provided parameters are passed as query bound parameters?

Whenever I see people using the old mysql_ functions on StackOverflow (which is, sadly, way too frequently) I generally tell people that prepared statements are the Chuck Norris (or Jon Skeet) of SQL injection security measures.

However, I've never actually seen any documentation that categorically states "this is 100% safe". My understanding of them is that they separate the query language and parameters all the way to the front door of the server, which then treats them as separate entities.

Am I correct in this assumption?

Polynomial
  • 132,208
  • 43
  • 298
  • 379
  • Kernel, database, and other components are not invulnerable to all possible attacks. Security teams contracted by finance, government, infrastructure, military, and critical business systems frequently require multiple security layers. Peruse a few bug lists and find holes that were patched ... after unspecified damage was done. To assume they "got'em all" is to deny statistical realities of software. It is not possible, with the finite test suite of a component, to test all possible attacks to the overall system containing that component. Safety is not an absolute 0.0 or 1.0 probability. – Douglas Daseeco Jan 19 '17 at 16:40

2 Answers2

60

Guarantee of 100% safe from SQL injection? Not going to get it (from me).

In principle, your database (or library in your language that is interacting with the db) could implement prepared statements with bound parameters in an unsafe way susceptible to some sort of advanced attack, say exploiting buffer overflows or having null-terminating characters in user-provided strings, etc. (You could argue that these types of attacks should not be called SQL injection as they are fundamentally different; but that's just semantics).

I have never heard of any of these attacks on prepared statements on real databases in the field and strongly suggest using bound parameters to prevent SQL injection. Without bound parameters or input sanitation, its trivial to do SQL injection. With only input sanitation, its quite often possible to find an obscure loophole around the sanitation.

With bound parameters, your SQL query execution plan is figured out ahead of time without relying on user input, which should make SQL injection not possible (as any inserted quotes, comment symbols, etc are only inserted within the already compiled SQL statement).

The only argument against using prepared statements is you want your database to optimize your execution plans depending on the actual query. Most databases when given the full query are smart enough to do an optimal execution plan; e.g., if the query returns a large percentage of the table, it would want to walk through the entire table to find matches; while if its only going to get a few records you may do an index based search [1].

EDIT: Responding to two criticisms (that are a tad too long for comments):

First, as others noted yes every relational database supporting prepared statements and bound parameters doesn't necessarily pre-compile the prepared statement without looking at the value of the bound parameters. Many databases customarily do this, but its also possible for databases to look at the values of the bound parameters when figuring out the execution plan. This isn't a problem, as the structure of the prepared statement with separated bound parameters, makes it easy for the database to cleanly differentiate the SQL statement (including SQL keywords) from data in bound parameters (where nothing in a bound parameter will be interpreted as an SQL keyword). This is not possible when constructing SQL statements from string concatenation where variables and SQL keywords would get intermixed.

Second, as the other answer points out, using bound parameters when calling an SQL statement at one point in a program will safely prevent SQL injection when making that top-level call. However, if you have SQL injection vulnerabilities elsewhere in the application (e.g., in user-defined functions you stored and run in your database that you unsafely wrote to construct SQL queries by string concatenation).

For example, if in your application you wrote pseudo-code like:

sql_stmt = "SELECT create_new_user(?, ?)"
params = (email_str, hashed_pw_str)
db_conn.execute_with_params(sql_stmt, params)

There can be no SQL injection when running this SQL statement at the application level. However if the user-defined database function was written unsafely (using PL/pgSQL syntax):

CREATE FUNCTION create_new_user(email_str, hashed_pw_str) RETURNS VOID AS
$$
DECLARE
   sql_str TEXT;
BEGIN
     sql_str := 'INSERT INTO users VALUES (' || email_str || ', ' || hashed_pw_str || ');'
     EXECUTE sql_str;
END;
$$
LANGUAGE plpgsql;

then you would be vulnerable to SQL injection attacks, because it executes an SQL statement constructed via string concatenation that mixes the SQL statement with strings containing the values of user defined variables.

That said unless you were trying to be unsafe (constructing SQL statements via string concatenation), it would be more natural to write the user-defined in a safe way like:

CREATE FUNCTION create_new_user(email_str, hashed_pw_str) RETURNS VOID AS
$$
BEGIN
     INSERT INTO users VALUES (email_str, hashed_pw_str);
END;
$$
LANGUAGE plpgsql;

Further, if you really felt the need to compose an SQL statement from a string in a user defined function, you can still separate data variables from the SQL statement in the same way as stored_procedures/bound parameters even within a user defined function. For example in PL/pgSQL:

CREATE FUNCTION create_new_user(email_str, hashed_pw_str) RETURNS VOID AS
$$
DECLARE
   sql_str TEXT;
BEGIN
     sql_str := 'INSERT INTO users VALUES($1, $2)'
     EXECUTE sql_str USING email_str, hashed_pw_str;
END;
$$
LANGUAGE plpgsql;

So using prepared statements is safe from SQL injection, as long as you aren't just doing unsafe things elsewhere (that is constructing SQL statements by string concatenation).

dr jimbob
  • 38,768
  • 8
  • 92
  • 161
  • Slightly off the security side, but SQL server provides option (recompile) to force a new compilation and it will use the current parameter values when choosing a plan. Although I don't know for sure, I expect other RDBMS provide a similar function. So no excuse for not using parameterised queries! – pipTheGeek May 21 '12 at 17:52
  • 1
    Exactly what I'd figured, then. As close to 100% as you can guarantee, ignoring implementation exploits. – Polynomial May 21 '12 at 18:13
  • 1
    @pipTheGeek - Interesting. I use postgresql as my rdbms of choice (free software) and cannot find this option, and MySQL specifically says they do not support ['WITH RECOMPILE'](http://dev.mysql.com/doc/refman/5.6/en/faqs-stored-procs.html#qandaitem-B-4-1-15). I think you may be able to do something similar in postgres if you first wrote a stored procedure and then did a prepared statement calling it--but not sure it would recompile execution plan (would need to check). – dr jimbob May 21 '12 at 18:34
  • Sorry, I'm a bit dense today. How does Lateral SQL Injection (http://www.databasesecurity.com/dbsec/lateral-sql-injection.pdf) fit into this? – Bruce Ediger May 22 '12 at 15:45
  • 3
    @BruceEdiger - Lateral SQL injection in your link can't happen if you use prepared statements with bound parameters. In pseudocode, if one defines `sql_str = "SELECT name FROM all_objects WHERE created = ?"` and then did `prepared_stmt = db_cursor.prepare(sql_str)`, `prepared_stmt.execute_with_param(SYSDATE)`, you can't use SYSDATE to inject code to alter the execution plan as the execution plan was determined in the `prepare()` step independent of SYSDATE's value. (Unless there's another vulnerability). The lateral flaw is doing naive string processing to construct an SQL statement. – dr jimbob May 22 '12 at 16:10
  • @drjimbob That might actually be one of the most informative comments I've ever read on this site. Now, I'm off to go read about the implementation of SQL parsers... – Polynomial Sep 02 '12 at 09:19
  • @Polynomial, the existence of a kernel, database, or other bug is always > 0.0. (BruceEdiger's comment that SQL injection CAN'T HAPPEN is actually counterproductive. Peruse the bug list of the open source software in question and you will find security holes that have been patched. The bug report will not mention the attacks that successfully destroyed or acquired critical data prior to the release of the bug fix. Experienced security teams require at least two levels of defense for critical systems. Validation and/or sanitation is a common (and prudent) first level. – Douglas Daseeco Jan 19 '17 at 16:32
  • I'd like to add, if you're using stored procedures for all of your sql, wouldn't the execution plans already be optimized? So if you're using good database practices, prepared statements wouldn't really reduce your query's efficiency by much. – HumbleWebDev Aug 29 '17 at 14:22
  • "With bound parameters, your SQL query execution plan is figured out..." - no its not. PHP's MySQL extension uses client side binding. Oracle (by default) will delay plan generation until it sees the parameters. Prepared statements are good for security if they incorporate privilege seperation, or some other means to close off bypass of the prepared statement. – symcbean Jun 12 '18 at 13:01
  • @symcbean - Agreed. The answer slightly oversimplified things by stating the RDBMS prepares the execution plan before looking at parameters. This is often done, but not always. However, prepared statements/bound parameters should still be safe as the DB has enough information when constructing the query plan to clearly delineate between the SQL statement and variables (that an attacker could try to inject SQL commands into, but the DB knows to interpret as data not SQL keywords). – dr jimbob Jun 13 '18 at 14:57
  • @drjimbob - You said, "The lateral flaw is doing naive string processing to construct an SQL statement." So, is there any way to protect against ignorant programmers (because that's the real problem, right)? – MicroservicesOnDDD Oct 13 '21 at 01:30
  • 1
    @MicroservicesOnDDD "So, is there any way to protect against ignorant programmers" Of course not, with the possible exceptions of having less ignorant programmers review their code, good style guide implementing best practices (written by less ignorant), having a lot of testing (both automated and humans pen-testing). – dr jimbob Oct 13 '21 at 15:45
27

100% safe? Not even close. Bound parameters (prepared statement-wise or otherwise) effectively can prevent, 100%, one class of SQL injection vulnerability (assuming no db bugs and a sane implementation). In no way do they prevent other classes. Note that PostgreSQL (my db of choice) has an option to bind parameters to ad hoc statements which saves a round trip regarding prepared statements if you don't need certain features of these.

You have to figure that many large, complex databases are programs in themselves. The complexity of these programs varies quite a bit, and SQL injection is something that has to be looked out for inside programming routines. Such routines include triggers, user-defined functions, stored procedures, and the like. It is not always obvious how these things interact from an application level as many good dba's provide some degree of abstraction between the application access level and the storage level.

With bound parameters, the query tree is parsed, then, in PostgreSQL at least, the data is looked at in order to plan. The plan is executed. With prepared statements, the plan is saved so you can re-execute the same plan with different data over and over (this may or may not be what you want). But the point is that with bound parameters, a parameter cannot inject anything into the parse tree. So this class of SQL injection issue is properly taken care of.

But now we need to log who writes what to a table, so we add triggers and user-defined functions to encapsulate the logic of these triggers. These pose new issues. If you have any dynamic SQL in these, then you have to worry about SQL injection there. The tables they write to may have triggers of their own, and so forth. Similarly a function call might invoke another query which might invoke another function call and so forth. Each of these is independently planned of the main tree.

What this means is that if I run a query with a bound parameter like foo'; drop user postgres; -- then it cannot directly implicate the top-level query tree and cause it to add another command to drop the postgres user. However, if this query calls another function directly or not, it is possible that somewhere down the line, a function will be vulnerable and the postgres user will be dropped. The bound parameters offered no protection to secondary queries. Those secondary queries need to make sure they use bound parameters too to the extent possible, and where not, will need to use appropriate quoting routines.

The rabbit hole goes deep.

BTW for a question on Stack Overflow where this problem is apparent, see https://stackoverflow.com/questions/37878426/conditional-where-expression-in-dynamic-query/37878574#37878574

Also a more problematic version (due to limitation on utility statements) at https://stackoverflow.com/questions/38016764/perform-create-index-in-plpgsql-doesnt-run/38021245#38021245

Chris Travers
  • 429
  • 4
  • 5
  • 2
    Which is why i prefer to keep the database as simple as possible. – Cees Timmerman Jul 06 '16 at 12:15
  • "Note that PostgreSQL (my db of choice) has an option to bind parameters to ad hoc statements which saves a round trip regarding prepared statements if you don't need certain features of these." Could you please share where this is documented? I tried to find it but with no success. Reason for looking: I want to add it to https://github.com/mauricio/postgresql-async to be able to bind parameters without allocating a prepared statement on the server. thx a lot! – Dominik Dorn Mar 20 '17 at 19:03
  • The documentation is a little difficult to understand. See the https://www.postgresql.org/docs/9.6/static/protocol-overview.html and the idea of "unnamed portals." A good place to look might be in the libpq source with the PQExecParams() function. It looks like this is handled internally (at least to me) like an unnamed prepared statement with the bind parameters sent in at the same time (later message, same packet?) – Chris Travers Mar 21 '17 at 08:03