Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 

K. Brian Kelley - Databases, Infrastructure, and Security

IT Security, MySQL, Perl, SQL Server, and Windows technologies.

SQL Injection - Why I Don't Think Parameterization is Enough

Note: Since there have been several comments on this, I'm using parameterization at the application layer in the security sense of using the CreateParameter method. I'm not talking about parameterized queries with respect to execution plans or the specific use of sp_executesql. I thought I made that clear with me saying "proper parameterization at the application layer" but since there have been several comments on that, it must not be.

One of the main defenses touted against SQL injection attacks is to use proper parameterization at the application layer. But while this gets most of the cases, there are clearly examples where this alone fails. For instance, consider the stored procedure:

CREATE PROC dbo.usp_ExecuteSQL
  
@SQL NVARCHAR(4000
)
AS
BEGIN
  EXEC
(@SQL
);
END;

Now I know the natural response by most folks is, "I would never see that in a production application." Perhaps not. But I have. In one organization's main application I found this procedure not once, but twice (albeit with different names). There was a standard in place that all database access from the application had to be done through stored procedures. The standard was met. But naturally because of such access, you can imagine what the permissions looked like within the database. And you can imagine the abuse that could have been performed through these types of stored procedures.

I know this is an extreme example, but it reflects that just focusing on parameterization isn't enough. For this stored procedure, the parameter would be defined properly, but that wouldn't stop SQL code from running in the back-end. Now I know the argument is, "Well, it's because the stored procedure used dynamic SQL." True enough. However, there are lots of cases out there where dynamic SQL solutions are running in production. For instance, I've seen things like the following:

USE [AdventureWorks2008];
GO

CREATE PROC 
dbo.usp_ReturnPeople
  
@Sort NVARCHAR(100
)
WITH EXECUTE AS 
SELF
AS
BEGIN
  DECLARE 
@SQL NVARCHAR(MAX
);
  
  
SET @SQL 
'SELECT Title, FirstName, MiddleName, LastName, Suffix  
              FROM person.Person
              ORDER BY ' 
@Sort
;
  
EXEC (@SQL
);
END

This stored procedure falls prey to situations like where @Sort can be sent in as '1;DELETE FROM person.Person;' which would effectively delete all the data in that table. And since @Sort would pass the parameterization check, using parameters doesn't serve as an effective control to prevent the SQL injection. Now I know there are techniques involving XML or using the CASE statement in the ORDER BY clause, but these aren't immediately obvious. As a result, it is possible to see an example like I've given above than those solutions. And yes, one could pare down the size of the parameter and that would help greatly, but when folks are trying to be flexible, these things aren't necessarily on their radar. Nor would a consideration be made to use EXECUTE AS with a defined user account that only has SELECT permissions against the table. And unless there is stringent code review, these types of things may slip into production and once they are there, they have to stay there until there is time to do another build.

Therefore, if we rely strictly on parameterization as our defense, we can still be beaten, even without someone working maliciously on the back-end code. Both examples I've given were written by well-meaning people who were trying to develop a reasonable solution. And the solutions they came up with solved their problems and they moved on. They didn't consider the security ramifications. Maybe they weren't aware enough to them to code defensively against them. Or maybe they were under a stringent deadline and were trying to get the job done. Whatever the situation, the vulnerability is there. So how do we guard against that sort of thing? Two things come to mind:

  1. Run scans against the code quickly looking for key words like EXEC or EXECUTE. That would have flagged both of these stored procedures. The issue then is whether or not there would have been time to get these stored procedures rewritten to meet a deadline, if there was one. However, if it was early enough in the cycle, such automated code searches would have caught both examples. And this is the perfect time to look for other bits of problem code like when we see CURSOR or a SELECT *.
  2. Perform input validation at the application layer. This one is much harder because most key words are normal words so if you're dealing with a string value and you see the word select or delete, is that valid or not? However, in the case of the sorting stored procedure, a check for any of the tell-tale signs of an injection attack, like the use of characters such as a semi-colon or single quote or a greater than or less than sign could have caught this. Looking for these types of flags will help, but they aren't the total solution.

Now none of this is new. In fact, David Litchfield talked about second order SQL injection attacks at BlueHat and has published work on lateral SQL injection attacks in Oracle. The crux of all of this is to say there isn't one single vaccine to cure SQL injection. Parameterization is effective, yes, but a cure all it's not. There can be structures in the database that permit SQL injection attacks despite parameterization. And therefore, we must not stop at just using parameterization, but we must investigate further to ensure that we catch those vulnerabilities.

 

 

Comments

Posted by David Benoit on 15 May 2009

Brian - Great post. I also really appreciated the JumpStartTV video that you had on the subject as well.

Thanks for the information and keep up the encouragement.

Posted by Steve Jones on 15 May 2009

Excellent post, Brian!

Posted by Jay Bonk on 15 May 2009

I can’t help but laugh, one of the companies I worked for had purchased another company, along with that came a web site and of course the database, which we inherited.  

Our policy was no embedded SQL everything needed to be in procedures.  After much arguing, the new development staff agreed to use stored procedures.

They were done with their conversion in a couple of days.  Only problem, we they had one procedure – an exact duplicate of your first example.

Posted by K. Brian Kelley on 15 May 2009

Jay, said isn't it?

Posted by Robert Davis on 15 May 2009

At a company that I used to work for, we created an interface similar to your example above so that our account managers could run ad hoc queries against our system. They had an interface for typing in their query that then got executed against the databases (Exec sp_executesql @SQL).

The difference is that we did both of the things you suggested above plus we severely restricted the permissions. The interface connected via a specific account that had no permissions on the server or in the databases other than connecting and executing the one stored procedure. The stored procedure executed in the context of an account that only had permissions to select and nothing else.

So even if someone did inject something that we didn't catch, it would error out if it tried to do anything other than select data.

Posted by Brent Ozar on 17 May 2009

Dude, I literally saw one of these stored procs this morning in an application people pay for.  Crazy.  (And no, it's not by my employer, heh.)

Posted by Phil Factor on 19 May 2009

Good blog. I've seen the havoc caused by this sort of exploit too. It is impossible to over-emphasize the importance of security against malicious hackers.

Posted by niks_j143 on 20 May 2009

Its really a very good post.

Posted by creynolds on 20 May 2009

Very misleading title on this post.  What he's talking about is not parameterization at all.  Keep this up and people will stop reading this site.

Posted by Hammad Arshad on 20 May 2009

Nice post

Posted by K. Brian Kelley on 20 May 2009

creynolds, I'm not sure what you mean. The use of parameters (parameterization) has been touted as a viable solution to SQL injection for .NET development (aspnet101.com/.../tutorials.aspx). And it does an awful lot, don't get me wrong. But there are gaps where it breaks down. That's what this post was about. Or am I missing what you're saying?

Posted by David.Poole on 20 May 2009

I saw a tip for parameterized queries used sp_executesql rather than EXEC.

The query was built up as a string complete with parameter

SELECT * FROM dbo.mytable WHERE MyValue = @Value

The parameter definition and value was passed into sp_executeSQL to be substituted in.

Worked like a dream and you get a cached execution plan.

Posted by JMasciantoni on 20 May 2009

Excellent starter discussion as well as a resource for more information.  I am a developer and not a DBA so it helps me to understand the DBA's concern and reinforces the way I've been structuring my SPs.  I was lucky to have a good example of this way back when I started.  Thanks again.

Posted by Simon Elliston Ball on 20 May 2009

@Robert Davis

Don't forget that data writing / deleting queries are not the only injection. A smart injector could well view data they are not meant to, or in the classic web example achieve a login they are not meant to by using their select privileges and changing the condition.

Posted by Phil Pledger on 20 May 2009

A few issues with this article:

1) Very misleading title.  This is NOT what is meant by parameterization.  sp_executesql has actual parameters.

2) You should NEVER execute a string that is built up from concatenating user input, no matter how good of a job you think you have done at sanitizing the input.

3) Searching for EXEC will generate a lot of false alarms as it will also hit on any place where a stored procedure is called.  You are looking for EXEC( not EXEC.

Posted by K. Brian Kelley on 20 May 2009

Phil,

1)  I'm speaking parameterization at the application layer. By using .CreateParameter() folks assume they are totally safe. Can you explain why you say it's not parameterization?

2) I would agree, but reality shows people do it. That was the point I was trying to make. We can keep our head in the sand and say there is no problem because we've dealt with all the issues since we used .CreateParameter() or look for the issue proactively and address it.

3) True enough, "EXEC(" is what should be used. Or actually a regex which discounts spaces between EXEC and the open paranthesis.

Posted by LC on 20 May 2009

I think creynolds has a point.  The title is a little misleading in that using parameterization and calling stored procs are entirely different methods of executing sql code.  They are in fact points of constant debate on how to manage your sql code, either in code using sp_executesql or in the database using stored procs.  The title implies a discussion around parameterized sql(from BOL):

EXECUTE sp_executesql

         N'SELECT * FROM AdventureWorks.HumanResources.Employee

         WHERE ManagerID = @level',

         N'@level tinyint',

         @level = 109;

Still a good article on why you shouldn't take these kinds of shortcuts in stored procs, just not what the title implies.

LC

Posted by K. Brian Kelley on 20 May 2009

creynolds, Phil, Leon, see if the note at the top makes the discussion clear. Hopefully that explains why the title isn't misleading.

Posted by Scott Roberts on 20 May 2009

I have to agree with the other comments about the misleading title. Neither of the examples in this post are "parameterized queries".

The article would be more accurately titled "SQL Injection - Why I Don't Think Using Stored Procedures is Enough".

Posted by katedgrt on 20 May 2009

Brian, I really enjoyed the article and I don't think it was misleading myself.  It was a good treatment of a topic that never can get enough attention.  I have seen all too many examples of wide open gaps like these in the short history of my consulting work.  

Posted by briansalentine on 20 May 2009

Good article.  Many, many,many times I've had to use dynamic SQL. Dynamic sort is one case. Also, dynamically adding join or where clauses to help with performance or to satisfy specific client requests.

The title is perfect, exactly what I expected to find.

Posted by Carleton on 20 May 2009

We have a case where we execute a sql command passed from the UI.  We've obtained our security "warm-and-fuzzys" by limiting the permissions similar to this:

USE [AdventureWorks2008];

GO

CREATE PROC dbo.usp_ReturnPeople

 @Sort NVARCHAR(100)

WITH EXECUTE AS SELF

AS

BEGIN

 DECLARE @SQL NVARCHAR(MAX);

 SET @SQL = 'SELECT Title, FirstName, MiddleName, LastName, Suffix  

             FROM person.Person

             ORDER BY ' + @Sort;

EXECUTE AS USER = 'VERY_Limited_User_Account';

EXEC (@SQL);

REVERT;

END

Posted by Clarence Garraway on 20 May 2009

I think I understand where creynolds is coming from, and I agree that the title is misleading.

Firstly, though, I must say that everything you've said is correct - it just doesn't address what I know as 'parameterisation'. You are addressing a fault of dynamic SQL.

My understanding of the term parameterisation in relation to sql injection attacks is where your query is harcoded, with all the variable parts represented by placeholders, not concatenated as in your example. Now in your particular example, there is no other way (I know of) to do what you're attempting to do. This is because SQL Server (to the best of my knowledge) only allows query variables to represent values, not column names and keywords. However, if I may, I think I can provide a similar query that looks like your example but will illustrate my point.

Consider the following code.

USE [AdventureWorks2008];

GO

CREATE PROC dbo.usp_ReturnPeople

 @firstName NVARCHAR(100)

WITH EXECUTE AS SELF

AS

BEGIN

 DECLARE @SQL NVARCHAR(MAX);

SET @SQL = 'SELECT Title, FirstName, MiddleName, LastName, Suffix  

           FROM person.Person

           where firstname =''' + @firstName + '''';

EXEC (@SQL);

END

Then, to continue your example, someone could do a sql injection by passing the @firstName as "1';DELETE FROM person.Person where title !='", sans my double quotes, of course.

However, this is not a parameterised query, as talked about in the article you referenced (aspnet101.com/.../tutorials.aspx). It is simply a hardcoded dynamic sql query created by concatenation. The parameterisation discussed in the article is where the sql is harcoded in ASP.NET, not the database, and .NET parameter objects are used to handle the substitution of the user's values into the query's body. That way, .NET gets a crack at the values first, and parses them for any funny stuff before sending the completed query to the database.

If you wanted to do the same thing at the database level, it might look like this.

USE [AdventureWorks2008];

GO

CREATE PROC dbo.usp_ReturnPeople

 @firstName NVARCHAR(100)

WITH EXECUTE AS SELF

AS

BEGIN

 DECLARE @SQL NVARCHAR(MAX), @prms nvarchar(100);

SET @SQL = N'SELECT Title, FirstName, MiddleName, LastName, Suffix  

           FROM person.Person

           where firstname = @firstName';

Set @prms = N'@firstName varchar(100)'

EXEC sp_executesql @SQL, @prms, @firstName

END

In conclusion, the problem you have described is real and your solution is valid. It just shouldn't have been called parameterisation. In particular, the problem exhibited by example you used cannot be solved by parameterisation anyway because you cannot write a parameterised query where the variable represents a column name.

Thanks for your time reading this. Happy coding!

Posted by Clarence Garraway on 20 May 2009

Ok, now I've read your note, I understand where you're coming from. I guess you're trying to say to people that simply using stored procedures, which must be passed parameters, isn't enough - the values for those parameters must be validated in some way to prevent injection attacks. Fair enough.

Posted by ben.mcintyre on 20 May 2009

Don't know what the fuss is about, really.

The point is that if you use

SELECT * FROM foo.Bar WHERE BarID= @value

you are in fact perfectly safe from SQL injection.

however if you use

EXEC('SELECT * FROM foo.Bar WHERE BarID=' + @value)

you are not.

Pretty simple really.  It's about embedded values versus concatenated values, and if you concatenate, you're really just passing the vulnerable non-parameterisation down a layer to the SQL while appearing to take care of it in the application.

Posted by rdeslonde on 22 May 2009

There problem is that any environment that has a stored procedure like that, will most likely not have the code in their application layer done properly either. So, in an environement where developers are diligent and know what they are doing, parameters are enough. There really is no other solution (that I know of). If your database is a mess, your in trouble any way you look at it.

Leave a Comment

Please register or log in to leave a comment.