How to cleanse (prevent SQL injection) dynamic SQL in SQL Server?

Question

We have a ton of SQL Server stored procedures which rely on dynamic SQL.

The parameters to the stored procedure are used in a dynamic SQL statement.

We need a standard validation function inside these stored procedures to validate these parameters and prevent SQL injection.

Assume we have these constraints:

1. We can't rewrite the procedures to not use Dynamic SQL

2. We can't use sp_OACreate etc., to use regular expressions for validation.

3. We can't modify the application which calls the stored procedure to validate the parameters before they are passed to the stored procedure.

Is there a set of characters we can filter out to ensure we are not susceptible to SQL injection?

Solution 1

I believe there are three different cases that you have to worry about:

• strings (anything that requires quotes): '''' + replace(@string, '''', '''''') + ''''
• names (anything where quotes aren't allowed): quotename(@string)
• things that cannot be quoted: this requires whitelisting

Note: Everything in a string variable (char, varchar, nchar, nvarchar, etc.) that comes from user-controlled sources must use one of the above methods. That means that even things you expect to be numbers get quoted if they're stored in string variables.

For more details, see the Microsoft Magazine _{(Obsolete link: 2016-10-19)}.

Here's an example using all three methods:

EXEC 'SELECT * FROM Employee WHERE Salary > ''' +
     REPLACE(@salary, '''', '''''') +   -- replacing quotes even for numeric data
     ''' ORDER BY ' + QUOTENAME(@sort_col) + ' ' +  -- quoting a name
     CASE @sort_dir WHEN 'DESC' THEN 'DESC' END     -- whitelisting

Also note that by doing all the string operations inline in the EXEC statement there is no concern with truncation problems. If you assign the intermediate results to variables, you must make sure that the variables are big enough to hold the results. If you do SET @result = QUOTENAME(@name) you should define @result to hold at least 258 (2 * 128 + 2) characters. If you do SET @result = REPLACE(@str, '''', '''''') you should define @result to be twice the size of @str (assume every character in @str could be a quote). And of course, the string variable holding the final SQL statement must be large enough to hold all the static SQL plus all of the result variables.

Solution 2

The trivial cases can be fixed by QUOTENAME and REPLACE:

set @sql = N'SELECT ' + QUOTENAME(@column) + 
   N' FROM Table WHERE Name = ' + REPLACE(@name, '''', '''''');

Although QUOTENAME may be used on literals too to add the single quotes and replace single quotes with double single quotes, because it truncates the input to 128 characters it is not recommended.

But this is just the tip of the iceberg. There are multipart names (dbo.table) you need to take proper care of: quoting the multipartname would result in an invalid identifier [dbo.table], it has to be parsed and split (using PARSENAME), then properly quoted into [dbo].[table].

Another problem is truncation attacks, which can happen even if you do the trivial REPLACE on literals, see New SQL Truncation Attacks And How To Avoid Them.

The issue of SQL Injection can never be solved with one magic function placed on every procedure. It is like asking 'I want a function that will make my code run faster'. Preventing injection attacks is and end-to-end game that requires coding discipline all the way through, it cannot be simply added as an afterthought. Your best chance is to inspect every single procedure and analyze the T-SQL code line-by-line, with an eye opened for vulnerabilities, then fix the problems as you find them.

Solution 3

With these constraints you are pretty screwed.

Here are a two options that might give you some direction:

1. Use white list validator/parser that only accept queries that are in a format and with keywords and tables that are expected. This will probably only work with a very good SQL parser that really understands the syntax.

2. Execute queries in a restricted environment. For instance, use a user account with very limited rights. For instance, only allow (read) access to certain views that will never return sensitive data and disallow access to all other views, all stored procedures, functions and tables. Even safer is to execute those queries on another database server. Also don't forget to disable the OPENROWSET command.

Please note the following:

1. When you accept all queries except those that have invalid keywords, you will definitely fail, because black listing always fails. Especially with such a complicated language as SQL.

2. Don't forget that allowing dynamic SQL from sources that you cannot trust is evil in its purest sense, even when you use these tips, because once in a while bugs are discovered that can be abused by sending specially crafted SQL to a server. Therefore, even if you apply these tips, the risk is still there.

3. When you decide to go with a solution that allows dynamic SQL. Please don't think you can come up yourself with a safe solution, especially if you're trying to protect sensitive business data. Hire a database server security specialist to help you with that.

Solution 4

Is there a set of characters we can filter out to ensure we are not susceptible to SQL injection?

NO

SQL injection is not called a "Certain Set Of Characters Injection", and for a reason. Filtering out certain character could complicate the particular exploit, but does not prevent SQL injection itself. To exploit an SQL injection one have to write SQL. And SQL is not limited to few special characters.

Solution 5

This is a really nasty problem, its not a problem you want to be solving, however here is a trivial case that works, (reviewers, please let me know if I missed a case, this comes with NO guarantees)

create proc Bad 
  @param nvarchar(500) 
as 

exec (N'select ''' + @param + N'''') 

go

-- oops injected
exec Bad 'help'' select ''0wned!'' select ''' 

go 

create proc NotAsBad
   @param nvarchar(500) 
as 

declare @safish nvarchar(1000), @sql nvarchar(2000) 
set @safish = replace(@param, '''', '''''')

set @sql = N'select ''' + @safish  + N''''

exec (@sql) 

go 

-- this kind of works, but I have not tested everything
exec NotAsBad 'help'' select ''0wned!'' select ''' 

View more solutions