SQL SERVER – One Trick of Handling Dynamic SQL to Avoid SQL Injection Attack?

SQL SERVER - One Trick of Handling Dynamic SQL to Avoid SQL Injection Attack? injection-500x500 SQL Server has so many things to learn and I always find it amazing. My conversations with customers often come up with security questions esp around SQL Injection. Many have claimed SQL Injection is a SQL Server problem. It takes quite some time for me to let them know there is nothing about SQL Server and SQL Injection. SQL Injection is an outcome of wrong coding practices. One of the recommendations I give is about not using Dynamic SQL. There might be some situations where you can’t avoid it. My only advice would be, avoid if possible. In this blog, I would demonstrate a SQL Injection problem due to dynamic SQL and a possible solution you can have.

Let’s assume that we have a simple search page where user can use the blank search or provide filter in any field. We have provided two fields to use “First Name” and “Last Name”. The user types something and hits search. Here is our code of stored procedure which fires behind the scene.

USE AdventureWorks2014
GO
CREATE PROCEDURE search_first_or_last
@firstName NVARCHAR(50)
,@lastName NVARCHAR(50)
AS
BEGIN
DECLARE @sql NVARCHAR(4000)
SELECT @sql = ' SELECT FirstName ,MiddleName, LastName' +
' FROM Person.Person WHERE 1 = 1 '
IF @firstName IS NOT NULL
SELECT @sql = @sql + ' AND FirstName LIKE ''' + @firstName + ''''
IF @lastName IS NOT NULL
SELECT @sql = @sql + ' AND LastName LIKE ''' + @lastName + ''''
EXEC (@sql)
END

If I use this string to execute in last name ”;drop table t1–

EXEC search_first_or_last '%K%', ''';drop table t1--'

The dynamic string would be

SELECT FirstName, MiddleName, LastName FROM Person.
Person WHERE 1 = 1 AND FirstName LIKE '%K%' AND LastName LIKE '';DROP TABLE t1--'

Do you see the problem? Yes, users can drop table t1 if code is running under a high privilege account.

One of the solution of the problem would be to use sp_executesql. Here is the better version using

CREATE PROCEDURE search_first_or_last
@firstName NVARCHAR(50)
,@lastName NVARCHAR(50)
AS
BEGIN
DECLARE @sql NVARCHAR(4000)
SELECT @sql = ' SELECT FirstName , MiddleName, LastName' +
' FROM Person.Person WHERE 1 = 1 '
IF @firstName IS NOT NULL
SELECT @sql = @sql + ' AND FirstName LIKE @firstName'
IF @lastName IS NOT NULL
SELECT @sql = @sql + ' AND LastName LIKE @lastName '
EXEC sp_executesql @sql
,N'@firstName nvarchar(50), @lastName nvarchar(50)'
,@firstName
,@lastName
END

Hope you would be able to use this and implement in your project. Are you using these simple techniques in your production code? Have you ever faced similar problems during audit? Do let me know of your learnings.

Reference: Pinal Dave (https://blog.sqlauthority.com)

MySQL, SQL Scripts, SQL Server, SQL Server Security
Previous Post
SQL SERVER – Invoking a Stored Procedure from Azure Mobile Services – Notes from the Field #066
Next Post
SQL SERVER – What are the Different Ways to Script Procedure with T-SQL

Related Posts

18 Comments. Leave new

  • Dear Dev,, I am beginner in SQL,,i read your posts,, these are very nice and informative. You are doing simply great…Farhan

    Reply
  • Pinal,
    with your instructions, i have created a stored procedure to check if it solves SQL injection problem. but in both the scenarioes I could see that SQL Servers gives following error

    Msg 3701, Level 11, State 5, Line 1
    Cannot drop the table ‘t1’, because it does not exist or you do not have permission.

    which means, even sp_executesql is trying to run ‘drop table command’

    Reply
    • have you created table t1 before trying it?

      Reply
    • It sounds like you’re calling the stored procedure from code, and using string concatenation to pass the parameters to the query.

      Stored procedures will not, and cannot, protect against this. As far as SQL is concerned, you’ve sent it two queries – one to run the stored procedure, and one to drop the table.

      The moral being, you should NEVER use string concatenation to build a SQL query. Use whatever facilities your data-access layer has to pass parameters as parameters.

      If your data-access layer doesn’t have the facility to pass parameters as parameters, and forces you to use string concatenation, then drop it and get a better one. :)

      Reply
      • In theory, what you say is completely right Richard. I am a big time supporter of using parameters and to avoid Dynamic SQL. If you have a search form with close to 20-25 parameters and some can be NULL, it becomes impossible to write such parameter based queries. A lot of developers still use this technique and you brought some great points. Thanks.

      • How does it become “impossible” to write a parameterised query in that situation? Any decent data-access layer will support as many parameters as the database can handle – for SQL Server, that’s 2100.

        For example, in .NET you would simply have 25 calls to “command.Parameters.AddWithValue(…)” – a reasonably large chunk of code, but hardly “impossible”.

        Within SQL itself, the approach you’ve demonstrated in this post will easily handle 25 possibly-null parameters.

  • shashibhushan shukla
    February 19, 2015 12:02 pm

    [dbo].[GetIDCardType] 0,0,10,’IDCardTypeID’,’DESC’,NULL
    ALTER PROCEDURE [dbo].[GetIDCardType] –0,0,10,’IDCardTypeID’,’ASC’,NULL
    — Add the parameters for the stored procedure here
    @IDCardTypeID int = 0,
    @StartIndex INT = 0,
    @TotalRecordsPerPage INT = 0,
    @SortColumn VARCHAR(100) = NULL,
    @SortDircetion VARCHAR(5) = NULL,
    @TotalRecords INT = NULL OUTPUT
    AS
    BEGIN

    DECLARE @EndIndex INT, @Sql NVARCHAR(MAX)
    SET @EndIndex = @StartIndex + @TotalRecordsPerPage
    SET @StartIndex = @StartIndex + 1

    SELECT @TotalRecords = COUNT(IDCardTypeID) FROM IDCardType

    if(@IDCardTypeID = 0)

    begin

    SET @Sql = ‘WITH CTE AS(SELECT IDCardTypeID, ROW_NUMBER() OVER (Order BY @SortColumn @SortDircetion)
    AS SrNo, IDCardTypeName,Description
    FROM IDCardType Where IsDeleted = 0)
    SELECT IDCardTypeID,IDCardTypeName, Description FROM CTE WHERE SrNo BETWEEN CAST(@StartIndex AS VARCHAR) AND CAST(@EndIndex AS VARCHAR)’

    EXEC sp_executesql @Sql ,N’@SortColumn VARCHAR(100),@SortDircetion VARCHAR(100),@StartIndex INT,@EndIndex INT’,
    @SortColumn,
    @SortDircetion,
    @StartIndex,
    @EndIndex
    PRINT @Sql
    end

    Error: Msg 102, Level 15, State 1, Line 1
    Incorrect syntax near ‘@SortDircetion’.

    Reply
    • Copy and paste what you get from Print @SQL. looks like a syntax error there.

      Reply
    • That’s one instance where you *can’t* use parameters. However, you should still validate the parameters:

      DECLARE @RealSortColumn sysname, @RealSortDirection varchar(4);

      SET @RealSortDirection = CASE @SortDirection
      WHEN ‘DESC’ THEN ‘DESC’
      ELSE ‘ASC’
      END;

      If @SortColumn Is Not Null
      BEGIN
      SELECT
      @RealSortColumn = QUOTENAME(name)
      FROM
      sys.columns
      WHERE
      object_id = OBJECT_ID(‘Basket.Baskets’)
      And
      name = @SortColumn
      ;
      END;

      If @RealSortColumn Is Null SET @RealSortColumn = ‘[DefaultSortColumn]’;

      SET @Sql = N‘WITH CTE AS
      (
      SELECT
      IDCardTypeID, ROW_NUMBER() OVER (ORDER BY ‘ + @RealSortColumn + N’ ‘ + @RealSortDirection + N’) AS SrNo,
      IDCardTypeName,
      Description
      FROM
      IDCardType
      WHERE
      IsDeleted = 0
      )
      SELECT
      IDCardTypeID,
      IDCardTypeName,
      Description
      FROM
      CTE
      WHERE
      SrNo BETWEEN CAST(@StartIndex AS VARCHAR) AND CAST(@EndIndex AS VARCHAR)
      ;’;

      EXEC sp_executesql @Sql,
      N’@StartIndex INT,@EndIndex INT’,
      @StartIndex,
      @EndIndex
      ;

      Reply
  • Hi Pinal thanks for sharing knowledge.
    I have a simple question here-how the person or injector will be knowing the name of the table in my database? :-), In our organization we simply follow some standard and one to prefix table name with some uncommon string like-OZO_TableName

    please share you thoughts on this.

    Reply
    • Once hacker can run queries under high privileges, its not difficult for him to get data from sys.objects or sys.tables.

      Reply
  • Greetings, very good ideas on doing dynamic SQL. I looked for ways on writing script for application searches and could not believe how some of the examples are done. Some of the UI design methods I have applied is to not let the user type any criteria, use objects with set values; then parameters is all that is passed. If requirements, or bad manager insists on user entry, search the input string for illeagle characters that injection would use and alert user they are not allowed. If you can’t control the UI end, clean up the parameter string in the proc replacing any injection characters with nothing.

    Reply
  • Good article. Is there a way to use IN clause in sp_executesql without causing SQL injection? Couldnt find an options and I’m using a table valued parameter now, filling the rows with the search values from the C# code and using something like this
    IF EXISTS (SELECT 1 FROM @stateCodes)
    BEGIN
    SET @sql = @sql + ‘ AND StateCode IN (SELECT SearchString FROM @stateCodes)’
    END
    EXEC sp_executesql @sql ,N’@stateCodes searchCriteria READONLY’, @stateCodes

    Is the right way to do to avoid any SQL injection?

    Reply
  • Hi Pinal,

    How to change the dynamic query in SELECT statement

    DECLARE @columnnames nvarchar(50),@statement nvarchar(max)
    SET @columnnames=’column1,column2,column3′
    SET @statement=’select ‘+@columnnames+’ from tablename’
    EXEC(@statement)

    Please suggest

    Reply
  • CREATE PROCEDURE dbo.[Test_PreventSQL_Injection](@UserInput varchar(max))
    AS
    BEGIN
    SET NOCOUNT ON;

    DECLARE @cmd nvarchar(max)

    SET @cmd = ‘SELECT Id ‘
    SET @cmd = @cmd + ‘FROM dbo.[TestTable] ‘
    SET @cmd = @cmd + ‘WHERE Kod IN (‘ + @UserInput + ‘)’

    EXEC sp_executesql @cmd, N’@UserInput varchar(max)’, @UserInput

    END
    GO

    EXEC dbo.[Test_PreventSQL_Injection] ‘1); DELETE dbo.[AnyTable]–‘

    Above execution will still delete data in table dbo.[AnyTable].

    Reply

Leave a Reply