#0202-SQL Server-Replacing EXECUTE with sp_executesql does not protect against SQL injection


There are many dangers that come from incomplete knowledge, or for that matter, from the lack of experimenting something on your own. Implementing a solution without proper understanding and testing is a sin that most of us developers have committed at some point (knowingly, or unknowingly).


One such point is ensuring that a system is protected from SQL injection. As the name suggests, SQL injection refers to the ability of an external attacker to inject a rogue SQL command to the database (e.g. dumping the contents of the database to the user) within a seemingly legitimate request. Allow me to demonstrate what I mean by this.


(Please note that the intention of this post is to bust a myth around protecting against SQL injection and is not intended to be a complete guide against SQL injection).


[EDIT: 2012-10-08, 23:10 IST: The examples in this post are intentially over simplified for a clear understanding of the concept. The situation described in this post does not generally mandate the use of dynamic queries. END EDIT]


SQL Injection – A demo


Assume that my application provides functionality to search for contacts stored in the system based on the last name. This requirement is realized in code, by the following stored procedure:

USE AdventureWorks2012
GO

–Safety check
IF OBJECT_ID(‘proc_SearchPersonByLastName’) IS NOT NULL
DROP PROCEDURE dbo.proc_SearchPersonByLastName
GO

–Create Test Procedure, prone to injection
CREATE PROCEDURE dbo.proc_SearchPersonByLastName
@filterString NVARCHAR(100)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @sqlstring NVARCHAR(200);
SET @sqlstring = ‘SELECT BusinessEntityID, PersonType, Title, FirstName, MiddleName, LastName, EmailPromotion
FROM Person.Person
WHERE LastName = ”’;

SET @sqlstring = @sqlstring + @filterString + ””;

BEGIN TRY
EXEC (@sqlstring);
END TRY
BEGIN CATCH
SELECT ERROR_MESSAGE(),
ERROR_NUMBER(),
ERROR_SEVERITY(),
ERROR_STATE();
–THROW is SQL 2012 specific!
–If you are running SQL 2008, please replace it with RAISERROR
–http://beyondrelational.com/modules/2/blogs/77/posts/11287/sunset-for-raiserror-and-sunrise-for-throw-sql-11-denali.aspx
THROW;
END CATCH
END
GO


What the stored procedure does is to append the filter criteria to the query and then uses EXEC or EXECUTE to execute the query. Here’s what can go wrong. Imagine that an attacker uses the following call:

–SQL Injection!
EXEC proc_SearchPersonByLastName ‘Hill” OR 1=1 OR 1=”%’
GO

The result? All contents of the table Person.Person are dumped out to the user because 1 will always be equal to 1 – something which the system is not supposed to do! This is a simple demonstration of a SQL injection attack. For more such examples, refer the links in the References section of this post.


Image showing that all records from Person.Person have been dumped to the user


(One of the) Solutions – the myth


The above is a classic example of SQL injection and one of the most common myths that surrounds SQL Injection is that it is due to the use of EXEC or EXECUTE. Some say – “simply replace EXEC with calls to sp_executesql and the system becomes SQL injection-proof”. This has to be the biggest misconception of all times around this subject.


Let’s simply replace EXEC with a call to sp_executesql and we see that SQL injection is still possible.

USE AdventureWorks2012
GO
–Alter Test Procedure, still prone to injection
ALTER PROCEDURE dbo.proc_SearchPersonByLastName
@filterString NVARCHAR(100)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @sqlstring NVARCHAR(200);
SET @sqlstring = ‘SELECT BusinessEntityID, PersonType, Title, FirstName, MiddleName, LastName, EmailPromotion
FROM Person.Person
WHERE LastName = ”’;

SET @sqlstring = @sqlstring + @filterString + ””;

BEGIN TRY
–Notice that we now use sp_executesql here
EXEC sp_executesql @sqlstring;
END TRY
BEGIN CATCH
SELECT ERROR_MESSAGE(),
ERROR_NUMBER(),
ERROR_SEVERITY(),
ERROR_STATE();
–THROW is SQL 2012 specific!
–If you are running SQL 2008, please replace it with RAISERROR
–http://beyondrelational.com/modules/2/blogs/77/posts/11287/sunset-for-raiserror-and-sunrise-for-throw-sql-11-denali.aspx
THROW;
END CATCH
END
GO

–Proof that injection is possible
EXEC proc_SearchPersonByLastName ‘Hill” OR 1=1 OR 1=”%’
GO


Simply replacing EXEC with sp_executesql does not prevent SQL injection


(One of the) Solutions – Parameterization


Using sp_executesql instead of EXEC is a partial solution. Using sp_executesql with parameters is the complete solution. Allow me to demonstrate. I have modified the stored procedure from above to use parameters:

USE AdventureWorks2012
GO
–Now, alter the SP to protect from injection
ALTER PROCEDURE dbo.proc_SearchPersonByLastName
@filterString NVARCHAR(100)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @sqlstring NVARCHAR(200);
DECLARE @params NVARCHAR(100);
SET @sqlstring = ‘SELECT BusinessEntityID, PersonType, Title, FirstName, MiddleName, LastName, EmailPromotion
FROM Person.Person
WHERE LastName = @personLastName’;
SET @params = ‘@personLastName NVARCHAR(100)’;

BEGIN TRY
EXEC sp_executesql @sqlstring,
@params,
@personLastName = @filterString;
END TRY
BEGIN CATCH
SELECT ERROR_MESSAGE(),
ERROR_NUMBER(),
ERROR_SEVERITY(),
ERROR_STATE();
THROW;
END CATCH
END
GO


What I have done is that I have explicitly specified that the query should expect a single parameter with a specific type – anything more is considered invalid and should not be processed. So, let me try it out:

USE AdventureWorks2012
GO
–Try injecting SQL code again – Won’t Work!
EXEC proc_SearchPersonByLastName ‘Hill” OR 1=1 OR 1=”%’
GO

sp_executesql with parameters protects agaisnt SQL injection


This call did not work because the statement expects just one parameter and is instead getting a complete condition – which is incorrect. The following correct call however, works:

–Proper call – success
EXEC proc_SearchPersonByLastName ‘Hill’
GO

sp_executesql with parameters protects agaisnt SQL injection


The conclusion that we can derive from this exercise is that to protect from SQL injection, one of the first things that need to be done is to use sp_executesql with parameterization. Simply replacing EXEC or EXECUTE calls with sp_executesql is not the solution.


References:



Until we meet next time,


Be courteous. Drive responsibly.

Advertisements

4 thoughts on “#0202-SQL Server-Replacing EXECUTE with sp_executesql does not protect against SQL injection

  1. marc_jellinek@hotmail.com

    An even better solution: Don’t put your query into strings to be executed later. Instead do this:

    USE AdventureWorks2012
    GO
    –Now, alter the SP to protect from injection
    ALTER PROCEDURE dbo.proc_SearchPersonByLastName
    @filterString NVARCHAR(100)
    AS
    BEGIN
    SET NOCOUNT ON;

    BEGIN TRY

    SELECT BusinessEntityID, PersonType, Title, FirstName, MiddleName, LastName, EmailPromotion
    FROM Person.Person
    WHERE LastName = @personLastName

    END TRY
    BEGIN CATCH
    SELECT ERROR_MESSAGE(),
    ERROR_NUMBER(),
    ERROR_SEVERITY(),
    ERROR_STATE();
    THROW;
    END CATCH
    END
    GO

    Like

    Reply
  2. Jeff Moden

    You beat me to it, Marc. I can unserstand that sometimes Dynamic SQL must be used, but this isn’t one of them.

    Still, hats off to Nakul for demonstrating that just changing EXEC to EXEC sp_executesql is not sufficient to prevent SQL Injection. He did say that this is “one of the solutions”.

    Like

    Reply
  3. Nakul Vachhrajani

    Thank-you, Marc & Jeff for your feedback. Yes, I tried to make the concept simple enough for all to understand, but perhaps I over-simplified things a bit. A comment has now been introduced warning readers about this.

    Thank-you, Jeff for the encouraging words in recognizing my efforts to bring some awareness around the methods to prevent SQL injection.

    Once again, thank-you very much for taking the time out, reading my posts and sharing your thoughts about them!

    Like

    Reply
  4. Quark

    Parameterization helps, but not all dynamic code can be parameterized. I find it’s also important to mention QUOTENAME() for when it can’t be – especially if it’s the table names that are dynamic.

    Like

    Reply

Let me know what you think about this post by leaving your feedback here!

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s