Introduction
Lately it seems like I am being bombarded by Dynamic SQL and people who insist it is the only way to accomplish something. Dynamic SQL is almost never the best way to accomplish a given task, but sometimes (and I do mean just sometimes) it is. In the post I will show why to use sp_ExecuteSql and how to convert an existing procedure to use it properly.
The Example
Below is an example stored procedure that I came up with that takes multiple parameters and based on those parameters formulates and executes a query. It is intentionally easy and could well be written otherwise but it works well for this purpose.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 | CREATE PROCEDURE dbo.GetTableInfo @object_id int = null, @name sysname = null AS DECLARE @sql_command nvarchar(max) SELECT @sql_command = N' SELECT [name], [object_id] FROM sys.tables ' IF @object_id IS NOT NULL AND [name] = ''' + @name + '''' PRINT @sql_command |
The stored procedure takes 2 parameters, @object_id and/or @name. Based on the inputs a WHERE clause is created and appended to the rest of the SQL statement. In case anyone tries to pull a fast one to see a full table list by submitting null for both arguments a check has been added to clear the string, causing nothing to be returned. At this point, the example looks fairly robust and somewhat bulletproof.
The Test(s)
Testing the stored procedure should be fairly simple. To keep things easy I will test in msdb by searching for the table sysjobs by name, object_id and then both. I will then test for the possibility of SQL Injection.
Test #1
EXEC dbo.GetTableInfo @name = ‘sysjobs’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [name] = ‘sysjobs’
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #2
EXEC dbo.GetTableInfo @object_id = 277576027
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [object_id] = 277576027
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #3
EXEC dbo.GetTableInfo @object_id = 277576027, @name = ‘sysjobs’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [object_id] = 277576027
AND [name] = ‘sysjobs’
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #4 – SQL Injection
EXEC dbo.GetTableInfo @name = ‘sysjobs” OR ”1”=”1”; SELECT name FROM sys.server_principals WHERE name != ”’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [name] = ‘sysjobs’ OR ’1′=’1′; SELECT name FROM sys.server_principals WHERE name != ”
name object_id
—————– —————
syssubsystems 5575058
sysproxysubsystem 21575115
restorefilegroup 30623152
.
.
.
(97 row(s) affected)
name
——————————-
sa
public
sysadmin
.
.
.
(33 row(s) affected)
The Problem
Testing looked great until the SQL Injection test. By mangling the parameters passed into the stored procedure I was able to get it to dump a list of all logins on the server. There is no reason that a call to a stored procedure could not be added in there instead followed by “–” to comment out the rest of the string. If xp_cmdshell were unsecured on this server the box would now belong to anyone that could exploit this vulnerability. This is about as bad as it gets short of leaving the sa password blank.
The Fix
In the example above I made a key error to prove a point. Even though I used sp_ExecuteSql, I used it in a way that was no better than using the EXEC statement. The proper way to use it is to build a parameterized SQL statement and pass the parameters into sp_ExecuteSql seperately. I know this sounds like a lot of extra work but an important thing to note is that all of the parameters can be passed for every call and will only used if they are in the SQL string that is passed in.
The added benefit of parameterized SQL is the elimination of procedure cache bloat. The example above creates a unique query string that will cause a plan to be added to the procedure cache every time the procedure is called unless the same exact parameters are passed. The fixed or improved version will add 3 plans to the procedure cache, slightly more if there are wide variations in the data but far less than the 1 for every call the example will add.
Here is the fixed version:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 | CREATE PROCEDURE dbo.GetTableInfo @object_id int = null, @name sysname = null AS DECLARE @sql_command nvarchar(max), @sql_parameters nvarchar(max) SELECT @sql_command = N' SELECT [name], [object_id] FROM sys.tables ', @sql_parameters = N'@object_id int, @name sysname' IF @object_id IS NOT NULL AND [name] = @name' PRINT @sql_command GO |
The Re-Test(s)
Testing the stored procedure should be fairly simple. To keep things easy I will test in msdb by searching for the table sysjobs by name, object_id and then both. I will then test for the possibility of SQL Injection.
Test #1
EXEC dbo.GetTableInfo @name = ‘sysjobs’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [name] = @name
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #2
EXEC dbo.GetTableInfo @object_id = 277576027
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [object_id] = @object_id
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #3
EXEC dbo.GetTableInfo @object_id = 277576027, @name = ‘sysjobs’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [object_id] = @object_id
AND [name] = @name
name object_id
—————– —————
sysjobs 277576027
(1 row(s) affected)
Test #4 – SQL Injection
EXEC dbo.GetTableInfo @name = ‘sysjobs” OR ”1”=”1”; SELECT name FROM sys.server_principals WHERE name != ”’
Returns:
SELECT [name],
[object_id]
FROM sys.tables
WHERE [name] = @name
name object_id
—————– —————
(0 row(s) affected)
Conclusion
Dynamic SQL should be considered a highly specialized tool of last resort and used in the way I have described. As I have shown, misuse can lead to your server and the data stored on it no longer belonging to you.
