Just Using sp_executesql Doesn’t Make Dynamic SQL Safe From SQL Injection

Safe Belt


A lot of people I’ve talked to about dynamic SQL have been under the misguided impression that just using sp_executesql will fix safety issues with SQL injection.

In reality, it’s only half the battle. The other half is learning how to act sober.

The gripes I hear about fully fixing dynamic SQL are:

  • The syntax is hard to remember (setting up and calling parameters)
  • It might lead to parameter sniffing issues

I can sympathize with both. Trading one problem for another problem generally isn’t something people get excited about.

Trading all the money in your company bank account to ransom your database probably isn’t something you’d get excited about either.

That’s not a very good lead on your rezoomay.

Holic


Here’s a trivial example:

CREATE TABLE dbo.DropMe(id INT);

DECLARE @DatabaseName sysname = N'';
SET @DatabaseName = N'S%'';DROP TABLE dbo.DropMe;--';

DECLARE @sql NVARCHAR(MAX) = N'
SELECT *
FROM sys.databases AS d
WHERE d.name LIKE ''%' + @DatabaseName + '%'';
';

PRINT @sql;
EXEC sys.sp_executesql @sql;

This will not only return a list of database names that contain S on my instance, but the printed SQL statement shows the whole string is executed.

SELECT *
FROM sys.databases AS d
WHERE d.name LIKE '%S%';DROP TABLE dbo.DropMe;--%';

Blue Flowers


The only way to not have that happen is to do this, and this is where people start complaining about remembering syntax:

CREATE TABLE dbo.DropMe(id INT);

DECLARE @DatabaseName sysname = N'';
SET @DatabaseName = N'S%'';DROP TABLE dbo.DropMe;--';

DECLARE @sql NVARCHAR(MAX) = N'
SELECT *
FROM sys.databases AS d
WHERE d.name LIKE ''%@iDatabaseName%'';
';

PRINT @sql;
EXEC sys.sp_executesql @sql, 
                       N'@iDatabaseName sysname', 
					   @iDatabaseName = @DatabaseName;

What prints out is this:

SELECT *
FROM sys.databases AS d
WHERE d.name LIKE '%@iDatabaseName%';

There’s also no search result returned, because no database is currently named ‘S%”;DROP TABLE dbo.DropMe;–‘.

But I get why people think this is annoying, because it is quirky at first.

  • If the string you use to encapsulate your parameters isn’t NVARCHAR, and/OR prefixed with N, you’ll get an error.
  • If you put your dynamic SQL variables on the wrong side of the equal sign, you’ll get an error.
  • And yes, if you’ve got skewed data, you’ll be more open to parameter sniffing.

The syntax stuff just takes a little getting used to, and performance stuff is often easier to fix than lost, stolen, or vandalized data.

Even if you’re real comfy with your backups, you’re still at risk of someone stealing confidential data.

Data Is A Liability


It’s really important that you review the personal data you collect to make sure it’s totally necessary.

It’s also really important for you to regularly archive data that you don’t actively need in your database.

For everything else, taking precautions like fixing unsafe dynamic SQL is just part of mitigating your data liabilities.

Thanks for reading!

Going Further


If this is the kind of SQL Server stuff you love learning about, you’ll love my training. I’m offering a 75% discount to my blog readers if you click from here. I’m also available for consulting if you just don’t have time for that and need to solve performance problems quickly.



4 thoughts on “Just Using sp_executesql Doesn’t Make Dynamic SQL Safe From SQL Injection

  1. Hi Erik,

    I have never used sysname for a variable, but am guessing that by doing so I am missing out on something Truly Fantasticâ„¢.

    What is the deal with sysname in non-system code? Or is it just a shortcut for demos?

Comments are closed.