87

I've been preaching both to my colleagues and here on SO about the goodness of using parameters in SQL queries, especially in .NET applications. I've even gone so far as to promise them as giving immunity against SQL injection attacks.

But I'm starting to wonder if this really is true. Are there any known SQL injection attacks that will be successfull against a parameterized query? Can you for example send a string that causes a buffer overflow on the server?

There are of course other considerations to make to ensure that a web application is safe (like sanitizing user input and all that stuff) but now I am thinking of SQL injections. I'm especially interested in attacks against MsSQL 2005 and 2008 since they are my primary databases, but all databases are interesting.

Edit: To clarify what I mean by parameters and parameterized queries. By using parameters I mean using "variables" instead of building the sql query in a string.
So instead of doing this:

SELECT * FROM Table WHERE Name = 'a name'

We do this:

SELECT * FROM Table WHERE Name = @Name

and then set the value of the @Name parameter on the query / command object.

Adam Bellaire
  • 108,003
  • 19
  • 148
  • 163
Rune Grimstad
  • 35,612
  • 10
  • 61
  • 76
  • we should clarify what is meant by parameters (as Jonathan Leffler pointed out) - I was thinking stored-procedure parameters, but there are also ? parms and {0} parms... – Steven A. Lowe Nov 20 '08 at 21:21
  • It's much easier to say, we don't use concatenation to build a query. –  Nov 21 '08 at 03:06
  • Since the tag is asp.net, I presume you are building web applications. In this case, you should also take care of XSS attacks, and maybe others – Spikolynn Jan 23 '09 at 17:18

9 Answers9

50

Placeholders are enough to prevent injections. You might still be open to buffer overflows, but that is a completely different flavor of attack from an SQL injection (the attack vector would not be SQL syntax but binary). Since the parameters passed will all be escaped properly, there isn't any way for an attacker to pass data that will be treated like "live" SQL.

You can't use functions inside placeholders, and you can't use placeholders as column or table names, because they are escaped and quoted as string literals.

However, if you use parameters as part of a string concatenation inside your dynamic query, you are still vulnerable to injection, because your strings will not be escaped but will be literal. Using other types for parameters (such as integer) is safe.

That said, if you're using use input to set the value of something like security_level, then someone could just make themselves administrators in your system and have a free-for-all. But that's just basic input validation, and has nothing to do with SQL injection.

Adam Bellaire
  • 108,003
  • 19
  • 148
  • 163
  • The key point is understanding the issue raised by Steve Lowe's answer, also pointed out in the article @mikekidder cites -- you have to be wary wherever the Dynamic SQL is, whether in the application or in the server. Dynamic SQL is dangerous - but can be made safe. – Jonathan Leffler Nov 20 '08 at 20:51
  • "there isn't any way for an attacker to pass data that will be treated like 'live' SQL". - This isn't quite true, see examples below. – Booji Boy Nov 20 '08 at 21:05
  • All the examples below are defining "parameterised query" to mean SQL code accepting parameters. The normal definition is a query that uses your DBMS parameters collection. Barring a DBMS bug, this latter technique prevents SQL injection. – HTTP 410 Nov 20 '08 at 21:39
  • 2
    I have read every single link. Please cite any link that refers to a working injection attack against the DBMS Parameters collection. Indeed, the link that you posted specifically refers to this approach as defeating SQL injection (see the "Using Type-Safe SQL Parameters" section). – HTTP 410 Nov 20 '08 at 23:20
  • Hi! Could you provide a link to Oracle SQL grammar or anything like that to prove that answer. I understand it and absolutely agree with you but it would be great to have official link to documentation, grammar etc BestRegards, Raimbek – Raimbek Rakhimbek Feb 14 '20 at 04:48
15

No, there is still risk of SQL injection any time you interpolate unvalidated data into an SQL query.

Query parameters help to avoid this risk by separating literal values from the SQL syntax.

'SELECT * FROM mytable WHERE colname = ?'

That's fine, but there are other purposes of interpolating data into a dynamic SQL query that cannot use query parameters, because it's not an SQL value but instead a table name, column name, expression, or some other syntax.

'SELECT * FROM ' + @tablename + ' WHERE colname IN (' + @comma_list + ')'
' ORDER BY ' + @colname'

It doesn't matter whether you're using stored procedures or executing dynamic SQL queries directly from application code. The risk is still there.

The remedy in these cases is to employ FIEO as needed:

  • Filter Input: validate that the data look like legitimate integers, table names, column names, etc. before you interpolate them.

  • Escape Output: in this case "output" means putting data into a SQL query. We use functions to transform variables used as string literals in an SQL expression, so that quote marks and other special characters inside the string are escaped. We should also use functions to transform variables that would be used as table names, column names, etc. As for other syntax, like writing whole SQL expressions dynamically, that's a more complex problem.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
12

There seems to be some confusion in this thread about the definition of a "parameterised query".

  • SQL such as a stored proc that accepts parameters.
  • SQL that is called using the DBMS Parameters collection.

Given the former definition, many of the links show working attacks.

But the "normal" definition is the latter one. Given that definition, I don't know of any SQL injection attack that will work. That doesn't mean that there isn't one, but I have yet to see it.

From the comments, I'm not expressing myself clearly enough, so here's an example that will hopefully be clearer:

This approach is open to SQL injection

exec dbo.MyStoredProc 'DodgyText'

This approach isn't open to SQL injection

using (SqlCommand cmd = new SqlCommand("dbo.MyStoredProc", testConnection))
{
    cmd.CommandType = CommandType.StoredProcedure;
    SqlParameter newParam = new SqlParameter(paramName, SqlDbType.Varchar);
    newParam.Value = "DodgyText";
    .....
    cmd.Parameters.Add(newParam);
    .....
    cmd.ExecuteNonQuery();
}
HTTP 410
  • 17,300
  • 12
  • 76
  • 127
  • Can you clarify what you mean by the DBMS Parameters collection as opposed to a procedure that accepts parameters? – Rune Grimstad Nov 20 '08 at 22:32
  • Rune, read the "Use Type-Safe SQL Parameters" section of this link: http://msdn.microsoft.com/en-us/library/ms161953.aspx – HTTP 410 Nov 20 '08 at 23:05
  • My response was to Rune's original question, before it was edited with the update. – mikekidder Nov 21 '08 at 00:10
  • I've read and reread that msdn-article on sql injection and I still don't see how there is a difference between the parameters a stored procedure takes and the parameters a dynamic query takes. Apart from the fact that dynamic queries are dynamic. You still have to bind the parameters, right? – Rune Grimstad Nov 21 '08 at 08:30
  • It's the binding that makes the difference. If you call a stored proc with parameters directly, no input filtering is done. But if you bind by (for example) by using the SqlCommand parameters collection in .NET, all of the parameters will be filtered and treated as plain text. – HTTP 410 Nov 21 '08 at 10:27
  • Thanks for the example! It was just what I was wondering about. :-) – Rune Grimstad Nov 21 '08 at 11:39
10

any sql parameter of string type (varchar, nvarchar, etc) that is used to construct a dynamic query is still vulnerable

otherwise the parameter type conversion (e.g. to int, decimal, date, etc.) should eliminate any attempt to inject sql via the parameter

EDIT: an example, where parameter @p1 is intended to be a table name

create procedure dbo.uspBeAfraidBeVeryAfraid ( @p1 varchar(64) ) 
AS
    SET NOCOUNT ON
    declare @sql varchar(512)
    set @sql = 'select * from ' + @p1
    exec(@sql)
GO

If @p1 is selected from a drop-down list it is a potential sql-injection attack vector;

If @p1 is formulated programmatically w/out the ability of the user to intervene then it is not a potential sql-injection attack vector

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Steven A. Lowe
  • 60,273
  • 18
  • 132
  • 202
  • No; the whole point is that the string passed to the DBMS is not part of the SQL statement. Therefore, the value in the string makes no difference to the interpretation of the SQL - just to the values referenced by the SQL. – Jonathan Leffler Nov 20 '08 at 20:12
  • That is how I see parameters as well. They are supposed to prevent this problem. – Rune Grimstad Nov 20 '08 at 20:13
  • 2
    Steven is right if for example you are passing a string into a sp that uses it to run something like sp_executeSql (sql server) then you still have a sql injection risk. – alexmac Nov 20 '08 at 20:24
  • @Steven: that is not a parameter to the SQL; you would have to have a placeholder (question mark) in place of the string concatenation. And SQL does not allow you to specify the table name by placeholder. That is a pure SQL injection vulnerability - the original problem. – Jonathan Leffler Nov 20 '08 at 20:37
  • @Steven: maybe the term 'parameter' has been overloaded once too often. :D – Jonathan Leffler Nov 20 '08 at 20:38
  • Despite what I said previously, this example is interesting. At the surface level, something is going to execute the stored procedure and pass the user input as a parameter to it, preferably via a placeholder. But that isn't the 'real SQL'. The advice is correct - but needs to be understood. – Jonathan Leffler Nov 20 '08 at 20:43
  • Continuing: it is crucial that people understand SQL injection fully (see http://xkcd.org/327) and understand how the vulnerability arises, and ensure that the DML statements are under control. You shouldn't do DDL at all, usually, not least because the statements don't take parameters. – Jonathan Leffler Nov 20 '08 at 20:45
  • So I definitely see why this is vulnerable, it's really no better than building your statement in code, can anybody think of WHY you would do this? It pretty much defeats the entire purpose of stored procs. The engine can't precompile and reuse intermediate code and there is little or no type checking...so it seems to me that there is zero benefit to this other than perhaps some level of decoupling that allows you to just call the proc from code rather than having it in the actual code – Chris Thompson Sep 15 '09 at 23:19
  • @[Chris Thompson]: the above example is contrived and overly-simplistic; no sane programmer would ever do that...so of course, someone somewhere will do exactly that ;-). Seriously, more complex circumstances can arise where dynamic construction of sql is the "best" solution, you just have to be very very careful to sanitize any parts of the dynamic sql that can originate from user input. – Steven A. Lowe Sep 16 '09 at 02:09
6

A buffer overflow is not SQL injection.

Parametrized queries guarantee you are safe against SQL injection. They don't guarantee there aren't possible exploits in the form of bugs in your SQL server, but nothing will guarantee that.

Blorgbeard
  • 101,031
  • 48
  • 228
  • 272
2

Your data is not safe if you use dynamic sql in any way shape or form because the permissions must be at the table level. Yes you have limited the type and amount of injection attack from that particular query, but not limited the access a user can get if he or she finds a way into the system and you are completely vunerable to internal users accessing what they shouldn't in order to commit fraud or steal personal information to sell. Dynamic SQL of any type is a dangerous practice. If you use non-dynamic stored procs, you can set permissions at the procesdure level and no user can do anything except what is defined by the procs (except system admins of course).

HLGEM
  • 94,695
  • 15
  • 113
  • 186
  • so the lesson here is that if you must use dynamic sql, only do so inside of a stored procedure. +1 good advice! – Steven A. Lowe Nov 20 '08 at 20:31
  • 1
    No -- dynamic SQL in stored procs can still introduce SQL injection flaws, by interpolating unvalidated data into the dynamic query. – Bill Karwin Nov 20 '08 at 20:37
  • No the lesson here is to never use dynamic SQl – HLGEM Nov 20 '08 at 21:16
  • @HLGEM - right, and automobiles are involved in traffic accidents, so we should never use automobiles. – Bill Karwin Nov 20 '08 at 21:29
  • But dynamic SQL in a stored proc runs (by default) with the permission of the caller, not like static SQL which runs with the permission of the stored proc owner. This is an important distinction. – HTTP 410 Nov 20 '08 at 21:32
1

Just remember that with parameters you can easily store the string, or say username if you don't have any policies, "); drop table users; --"

This in itself won't cause any harm, but you better know where and how that date is used further on in your application (e.g. stored in a cookie, retrieved later on to do other stuff.

nos
  • 223,662
  • 58
  • 417
  • 506
1

It is possible for a stored proc to be vulnerable to special types of SQL injection via overflow/truncation, see: Injection Enabled by Data Truncation here:

http://msdn.microsoft.com/en-us/library/ms161953.aspx

Booji Boy
  • 4,522
  • 4
  • 40
  • 45
  • If you read the article in detail, you'll see that using SQL Server's Parameters collection prevents this attack. And that's the normal definition of a "parameterised Query" - it uses the Parameters collection of the DBMS. – HTTP 410 Nov 20 '08 at 21:43
1

You can run dynamic sql as example

DECLARE @SQL NVARCHAR(4000);
DECLARE @ParameterDefinition NVARCHAR(4000);

SELECT  @ParameterDefinition = '@date varchar(10)'

SET @SQL='Select CAST(@date AS DATETIME) Date'

EXEC sp_executeSQL @SQL,@ParameterDefinition,@date='04/15/2011'
Mohamed Abbas
  • 138
  • 1
  • 11