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?

Sheepdog

ouch. normally it's 3) which should be modified to prevent SQL injection. Remember, it's "SQL Injection", not "SQL Rejection". Once it gets to the DB, it should already be cleansed. But if you say you can't change the app, then i guess you can't. Interested to see the answers. – Porett

Answer 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.

Atoll

I agree here, it totally depends on what SQL is being constructed – Voncile

Answer 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.

Myelitis

I recommend not using PARSENAME because it's intended to be used on already-quoted names. If your user tells you he wants to get data from secret..table you want to query against [secret..table] and get an error. You don't want him to be able to query [secret]..[table]! – Atoll

In my opinion, running any dynamic SQL using anything other than sp_executesql with all values passed as parameters is just pure malpractice. – Laodicean

Answer 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.

Hoey

Answer 4

OWASP has some information on this strategy. It should always be a last-ditch option (as explained in the article I'm linking to) but if it's your only option...

http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet

a quote from the article about it being a last-ditch option

However, this methodology is frail compared to using parameterized queries. This technique should only be used, with caution, to retrofit legacy code in a cost effective way. Applications built from scratch, or applications requiring low risk tolerance should be built or re-written using parameterized queries.

In essence, the argument against this approach is even if you do escape all the known bad input, there's no guarantee that someone won't come up with a way to circumvent it inthe future.

However, to answer your question specifically...

a list of characters to escape is in the article I linked to above.

Edit As noted, the article doesn't provide very good links. However, for SQL Server, this one does: http://msdn.microsoft.com/en-us/library/ms161953.aspx

Note that the list of characters you need to escape will vary based on the DB platform, but it looks like you're using SQL Server, so this should be relevant..

Quote from the article below:

Filtering input may also be helpful in protecting against SQL injection by removing escape characters. However, because of the large number of characters that may pose problems, this is not a reliable defense. The following example searches for the character string delimiter.

private string SafeSqlLiteral(string inputSQL)
{
  return inputSQL.Replace("'", "''");
}

LIKE Clauses

Note that if you are using a LIKE clause, wildcard characters still must be escaped:

s = s.Replace("[", "[[]");
s = s.Replace("%", "[%]");
s = s.Replace("_", "[_]");

Worthless

-1: The article doesn't say what characters to escape for MS SQL Server. It just links to another article that doesn't make it obvious which characters to escape. – Atoll

Answer 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 ''' 

Voncile

+1, I've never seen anything to suggest that this doesn't work. – Atoll

In my opinion, running any dynamic SQL using anything other than sp_executesql with all values passed as parameters is just pure malpractice. – Laodicean

Still vulnerable. Suppose that the body of NotAsBad contains the following: set @sql = N'select * from ' +@safish ....if the user can guess the name of a table they can submit @param = 'tablename; drop database xyz; --' – Sheepdog

@Sheepdog this works fine for the trivial case, of course depending on your context you need to escape sql in different ways, hence the warning against doing this, I agree with @KM, in general this is something like this is a bad idea and not a problem you want to have to solve – Voncile

Answer 6

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.

Pardoes

Answer 7

Can you get SQL CLR can be of great use -- you can at least use it to write much, much more effective sanitization than one can do using T-SQL. In a perfact world, you can replace the stored procs completely with parameterized statements and other stronger structures.

Ophthalmia

unfortunately, I cannot use CLR due to DBA restrictions – Sheepdog

Answer 8

There is another approach that may possibly work, although it depends on what characters are allowed in the stored procedure's parameters. Instead of escaping the troublesome characters that can be used for SQL injection, delete the characters instead. Eg if you have this SP:

create procedure dbo.MYSP(@p1 varchar(100))
as begin
  set @p1 = Replace(@p1, '''',' '); -- Convert single quotes to spaces
  set @p1 = Replace(@p1, ';', ' ');
  set @p1 = Replace(@p1, '--', ' ');      
  set @p1 = Replace(@p1, '/*', ' ');      
  set @p1 = Replace(@p1, '*/', ' ');      
  set @p1 = Replace(@p1, 'xp_', ' ');      
  ...
end;

you can replace any single quotes with spaces or with an empty string. This approach can also be used to replace comment characters such as /* */ -- by using more Replace commands (as I have just shown above). But note this approach will only work if you never expect these characters in normal input, and this depends on your application.

Note the set of replaced characters is based on https://msdn.microsoft.com/en-us/library/ms161953(SQL.105).aspx

Formalin

SQL injection is not called a "Single Quote injection". For a reason. – Pardoes

I am not familiar with 'single quote injection', the technique I just described is one method of guarding against SQL Injection and it's based on the Microsoft article I referenced above. I am unclear why you down-voted this answer. – Formalin

I am always keen to learn more about security, and I would welcome your explanation of why Microsoft's recommendation in msdn.microsoft.com/en-us/library/ms161953(SQL.105).aspx is 'deliberately flawed'. – Formalin

Because if this recommendation were used on this site for example, you would be unable to post your answer at all. – Pardoes

Again I'm trying to understand here - is it your view that Microsoft's recommendation is pathetic? To me it seemed one approach that might help the original question, bearing in mind all the constraints they listed in the question. – Formalin

Besides, it does not prevent SQL injection. It could complicate the exploit, but it has absolutely nothing to do with the injection itself. – Pardoes