8

The following is a dynamic filtering solution that uses sp_executesql

IF OBJECT_ID(N'dbo.GetOrders', N'P') IS NOT NULL DROP PROC dbo.GetOrders;
GO
CREATE PROC dbo.GetOrders
 @orderid AS INT = NULL,
 @custid AS INT = NULL,
 @empid AS INT = NULL,
 @orderdate AS DATE = NULL
AS
DECLARE @sql AS NVARCHAR(1000);
SET @sql = 
 N'SELECT orderid, custid, empid, orderdate, filler'
 + N' /* 27702431-107C-478C-8157-6DFCECC148DD */'
 + N' FROM dbo.Orders'
 + N' WHERE 1 = 1'
 + CASE WHEN @orderid IS NOT NULL THEN
 N' AND orderid = @oid' ELSE N'' END
 + CASE WHEN @custid IS NOT NULL THEN
 N' AND custid = @cid' ELSE N'' END
 + CASE WHEN @empid IS NOT NULL THEN
 N' AND empid = @eid' ELSE N'' END
 + CASE WHEN @orderdate IS NOT NULL THEN
 N' AND orderdate = @dt' ELSE N'' END;
EXEC sp_executesql
 @stmt = @sql,
 @params = N'@oid AS INT, @cid AS INT, @eid AS INT, @dt AS DATE',
 @oid = @orderid,
 @cid = @custid,
 @eid = @empid,
 @dt = @orderdate;
GO

On p 541 of T-SQL Querying, it says

Because the dynamic code uses parameters rather than injecting the constants into the code, it is not exposed to SQL injection attacks.

How does the use of parameters in sp_executesql protect against SQL injection?

Thank you

0

3 Answers 3

13

double-up

To answer your question, you need to experiment with alternatives to using sp_executesql with parameters:

  • Using EXEC (without sp_executesql)
  • Using sp_executesql (without parameters)

Both of which can lead to SQL injection attacks, under the right circumstances.

It's probably worth noting that even totally unparameterized, the code above is relatively low-risk, since the data types being passed are not string types, but it's still possible.

Strings carry a much higher risk of a malicious payload.

The code examples below are from my presentation about using dynamic SQL in a different context, but apply pretty well to your question.

string-safe

You can use code like this safely, because the user input isn't part of the string that gets executed:

DECLARE 
    @SQLString nvarchar(MAX) = N'',
    @TableName sysname = N'Votes';

IF @TableName = N'Votes'
BEGIN
    SET @SQLString += N'SELECT COUNT_BIG(*) AS records FROM dbo.Votes AS v;'
END

IF @TableName = N'Posts'
BEGIN
    SET @SQLString += N'SELECT COUNT_BIG(*) AS records FROM dbo.Posts AS p;'
END

EXEC(@SQLString);
GO 

unsafe-strings

In this example, user input is concatenated into the string that gets executed, and isn't parameterized. This can cause problems:

DECLARE 
    @SQLString nvarchar(MAX) = N'',
    @Filter nvarchar(MAX) = N'',
    @Title nvarchar(250) = N''' 
  UNION ALL   
  SELECT 
      t.object_id, t.schema_id, t.name, SCHEMA_NAME(t.schema_id), t.create_date, t.modify_date, NULL 
  FROM sys.tables AS t --'; 
/* This ends the current statement, and adds in some sneaky code */

SET @SQLString += N' 
  SELECT TOP (5000) 
      p.OwnerUserId, p.Score, p.Tags, p.Title, p.CreationDate, p.LastActivityDate, p.Body 
  FROM dbo.Posts AS p 
  WHERE p.OwnerUserId = 22656 ';

/* This appends the sneaky code onto our harmless query */
IF @Title IS NOT NULL
BEGIN
    SET @Filter = @Filter + N' 
  AND p.Title LIKE ''' + N'%' + @Title + N'%''';
END;

IF @Filter IS NOT NULL
BEGIN
    SET @SQLString += @Filter;
END;

SET @SQLString += N' 
  ORDER BY p.Score DESC;';

/* Check the messages tab... */
RAISERROR('%s', 0, 1, @SQLString) WITH NOWAIT;

/* Check the results -- what's that at the end? */
EXEC (@SQLString);

The end result is a query that gets executed like so, which searches the Title column for a single wildcard, and then an additional result that lists all the tables in the database.

  SELECT TOP (5000) 
      p.OwnerUserId, p.Score, p.Tags, p.Title, p.CreationDate, p.LastActivityDate, p.Body 
  FROM dbo.Posts AS p 
  WHERE p.OwnerUserId = 22656  
  AND p.Title LIKE '%' 
  UNION ALL   
  SELECT 
      t.object_id, t.schema_id, t.name, SCHEMA_NAME(t.schema_id), t.create_date, t.modify_date, NULL 
  FROM sys.tables AS t --%' 
  ORDER BY p.Score DESC;

While many people will focus on memes like dropping tables, the real issue with dynamic SQL is usually theft of data. That's where the money is.

still-not-safe

Using sp_executesql is a good first step, but it still needs to be used with parameters. Code like below is still subject to SQL injection in the same way as above.

DECLARE 
    @SQLString nvarchar(MAX) = N'',
    @Filter nvarchar(MAX) = N'',
    @Title nvarchar(250) = N''' 
  UNION ALL   
  SELECT 
      t.object_id, t.schema_id, t.name, SCHEMA_NAME(t.schema_id), t.create_date, t.modify_date, NULL 
  FROM sys.tables AS t --';
/* This ends the current statement, and adds in some sneaky code */

SET @SQLString += N' 
  SELECT TOP (5000) 
      p.OwnerUserId, p.Score, p.Tags, p.Title, p.CreationDate, p.LastActivityDate, p.Body 
  FROM dbo.Posts AS p 
  WHERE p.OwnerUserId = 22656 ';

/* This appends the sneaky code onto our harmless query */
IF @Title IS NOT NULL
BEGIN
    SET @Filter = @Filter + N' 
  AND p.Title LIKE ''' + N'%' + @Title + N'%''';
END;

IF @Filter IS NOT NULL
BEGIN
    SET @SQLString += @Filter;
END;

SET @SQLString += N' 
  ORDER BY p.Score DESC;';

/* Check the messages tab... */
RAISERROR('%s', 0, 1, @SQLString) WITH NOWAIT;
/* Check the results -- what's that at the end? */
EXEC sys.sp_executesql 
    @SQLString;

The same query as above will be executed.

back-to-safety

Using code that lines up better with your example, we can avoid SQL injection by assigning the value to a parameter instead of concatenating it directly into the string.

DECLARE 
    @SQLString nvarchar(MAX) = N'',
    @Filter nvarchar(MAX) = N'',
    @Title nvarchar(250) = N''' 
  UNION ALL 
  SELECT 
      t.object_id, t.schema_id, t.name, SCHEMA_NAME(t.schema_id), t.create_date, t.modify_date, NULL 
  FROM sys.tables AS t --'; 
/* This ends the current statement, and adds in some sneaky code */

SET @SQLString += N' 
  SELECT TOP (5000) 
      p.OwnerUserId, p.Score, p.Tags, p.Title, p.CreationDate, p.LastActivityDate, p.Body 
  FROM dbo.Posts AS p 
  WHERE p.OwnerUserId = 22656 ';

/* This appends the sneaky code onto our harmless query */
IF @Title IS NOT NULL
BEGIN
    SET @Filter = @Filter + N' 
  AND p.Title LIKE N''%'' + @Title + N''%'' ';
END;

IF @Filter IS NOT NULL
BEGIN
    SET @SQLString += @Filter;
END;

SET @SQLString += N' 
  ORDER BY p.Score DESC;';

/* Check the messages tab... */
RAISERROR('%s', 0, 1, @SQLString) WITH NOWAIT;

/* Check the results -- what's that at the end now? */
EXEC sys.sp_executesql 
    @SQLString, 
  N'@Title NVARCHAR(250)', 
    @Title;

The results of our string-building are different this time. Now it looks like this:

  SELECT TOP (5000) 
      p.OwnerUserId, p.Score, p.Tags, p.Title, p.CreationDate, p.LastActivityDate, p.Body 
  FROM dbo.Posts AS p 
  WHERE p.OwnerUserId = 22656  
  AND p.Title LIKE N'%' + @Title + N'%'  
  ORDER BY p.Score DESC;

Rather than returning a result, which includes the details from sys.tables, we get zero rows back because the parameter is set to the search string, and no post titles match it.

next-next-next

Hopefully this gives you a better understanding of how parameterized dynamic SQL can help you avoid SQL injection attacks.

There are other good reasons to use it, like better plan caching, but that's outside the scope of this question.

For some more information, check out my posts here:

8

SQL injection is the act of taking the data of parameters that are used to create a query, and injecting them into the query. In other words, instead of using parameters orderid = @oid you are using orderid = 'abc'. This leaves you open to a malicious or accidental injection of an incorrect or unintended query. You might for example have an apostrophe in the parameter, or it may not translate dates correctly. Or more dangerously, a hacker tries something like Robert'); DROP TABLE Students;--.

The reason the above query is much safer is that it is essentially static. (This particular one is known as a Kitchen Sink Query and has other performance benefits.) It only changes in response to a limited set of conditions: whether or not a parameter is supplied. It does not inject the actual data into the query.


SET @sql = 
 N'SELECT orderid, custid, empid, orderdate, filler'
 + N' /* 27702431-107C-478C-8157-6DFCECC148DD */'
 + N' FROM dbo.Orders'
 + N' WHERE 1 = 1'

This part is static and is always part of the query


 + CASE WHEN @orderid IS NOT NULL THEN
 N' AND orderid = @oid' ELSE N'' END

This part is conditionally added. However, importantly, the actual data is not part of the query, only whether or not data is being supplied is the part that changes.

This is far easier to guard against, as the only risk now is minor syntax errors that you would know about up front (you obviously don't get Intellisense). The possible range of different queries is much smaller: you only have 16 different possibilities, and they all follow the same lines, so you can reason about the results much better, and can easily test that all possibilities work.


The upshot of it is that you get separation of query and data. The query is passed to the compiler, and separately the data is bound to the parameters, which act as placeholders. There is no possibility of the compiler misinterpreting the data as actual code, it always remains data.

1

Accepted answer is great from an explanation POV but thought there should be a simpler example of injection users can play with.

DECLARE @goodSQL nvarchar(100) = N'';
DECLARE @badSQL nvarchar(100) = N'';
DECLARE @maliciousInput nvarchar(100) = '''; PRINT ''SQL Injection''; PRINT ''Closing quote here'

-- Vulnerable | Params close existing statement to execute as normal commands
SET @badSQL = N'PRINT ''' + @maliciousInput + '''';
EXEC(@badSQL);

-- Output: SQL Injection

-- Protected | Params not interpreted
SET @goodSQL = N'PRINT @INPUT';
EXEC sys.sp_executesql
    @goodSQL,
    N'@INPUT nvarchar(max)', @maliciousInput;

-- Output: '; PRINT 'SQL Injection'; PRINT 'Closing quote here

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.