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)
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
thanks @fanni339
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’
have you created table t1 before trying it?
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. :)
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.
[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’.
Copy and paste what you get from Print @SQL. looks like a syntax error there.
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
;
NB: Replace “OBJECT_ID(‘Basket.Baskets’)” with “OBJECT_ID(‘IDCardType’)” – copy and paste error!
Thanks for your help Richard.
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.
Once hacker can run queries under high privileges, its not difficult for him to get data from sys.objects or sys.tables.
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.
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?
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
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].