Thursday, June 30, 2005 11:30 PM
Is Dynamic SQL in Your Stored Procedures Vulnerable to SQL Injection?
We all should be familiar with the fact that concatenating user input directly into SQL statements is an open invitation to an SQL Injection attack. Code such as
MySql = "Select * from Orders where Customer ID='" & txtCustomerId & "'"
should be avoided. If you need some more background information on SQL Injection attacks, I am building a reference at http://aspadvice.com/blogs/rjdudley/articles/2736.aspx. This reference will be updated as time goes on--there are a few good references now, and I'll post update notices to the security section of my blog at http://aspadvice.com/blogs/rjdudley/category/256.aspx.
The recommended practice for avoiding SQL Injection attacks is to use parameterized queries or stored procedures (sprocs), where user input is passed as parameters. Since information in parameters is not treated as executable code, any SQL code conatined in the user input is rendered harmless. Or is it? This depends on what you do with that input inside of your sproc.
One of the common functions on a web site is querying a data store. In advanced searches (those with more than a single input), it would be infeasible to create and mainatin an sproc for every combination of search critera. Instead, one practice is to create an sproc that dynamically creates the SELECT statement based on the parameters passed to it. Typically, there is an input parameter for each input on the search form, which is rendered optional by adding "=NULL" after the parameter declaration (e.g., @orderId int=NULL). Then, the sproc uses a series of statements such as
IF @orderId IS NOT NULL
select @sql = @sql + ' AND order_id=' + @orderId
to generate the complete SQL statement. At the end of the sproc, the EXECUTE statement is used to query the database using the dynamically generated SQL statement.
I remember what a revolutionary concept dynamic SQL in an sproc was for me when I was learning to write SQL. It opened up a whole new way of writing SQL code and handling advanced searches on my websites. But did you catch the security problem in the previous SQL statement? I didn't at first, and in fact, I've been making this same security mistake for some time now. It wasn't until I finally listened to Kim Tripp on DotNetRocks that I realized the problem (download the show from http://www.dotnetrocks.com/default.aspx?showID=75), and fortunately I only have a few sprocs to rewrite and fix this problem.
Look carefully at the statement again. It looks like the parameter is being used in the SQL statement, but in reality, the parameter's value is being concatenated to the SQL statement. The technique demonstrated above is no better than the technque we dismissed in the first paragraph.
After listening to Kim's show, I did some digging around, and found an excellent reference on how to handle dynamic SQL in search queries at http://www.sommarskog.se/dyn-search.html. In this article, Microsoft Sql Server MVP Erland Sommarskog details ways to use dynamic and static SQL to perform searches that have a number of possible combinations of inputs.
As Erland shows us, the correct way to use dynamic SQL in the situation I presented above is to concatenate another parameter into the SQL statement, as so:
IF @orderId IS NOT NULL
select @sql = @sql + ' AND order_id=@xorderId'
We then create a parameter list of these second parameters, as so:
SELECT @paramList = '@xorderId'
To finally execute the query, we execute a system sproc named sp_executesql. As Erland states:
sp_executesql is a system procedure with a very special parameter list. The first parameter is a parameterized SQL statement. The second parameter is a parameter-list declaration, very similar to the parameter list to a stored procedure. And the remaining parameters are simply the parameters in that parameter-list parameter.
Our final statement would end up looking like:
EXECUTE sp_executesql @sql, @paramList, @orderId
And with this technique, our query is safe from malicious user input. This whole process is outlined in detail in Erland's article.
Since writing sprocs as outlined in Erland's article can be tedious, I created a CodeSmith template that will do the work for you. You only need to input the table you wish to query, and CodeSmith will generate a complete sproc for you. You can then edit the sproc down, since it will include every column in the table. You can find the template at http://www.ericjsmith.net/codesmith/forum/default.aspx?f=9&m=4346.