The problem with SQL Injection is that most people don’t realise the fundamental concept which makes SQL Injection vulnerability not only easy to spot, but also easy to prevent. And it’s the thing that SQL Injection has in common with countless other hacking mechanisms that have been around since the early days of computing.
The simple truth is this: You are vulnerable if you confuse DATA with COMMAND.
SQL Injection occurs when a piece of user-entered data makes its way into the command that is being executed. Not simply as a piece of data, such as the value that a column is being compared to, but when it becomes something which actually gets executed. Buffer overflow attacks worked in the same way – although the method of getting the data to be executed is different.
So you should never put yourself in a situation where something that a user has entered gets executed. It doesn’t matter what sanitisation you have done, you should never do it. Creative hackers will be able to find ways around however you do your sanitisation. Just don’t run something that has come from the user – pass it in using parameters instead.
Code within a stored procedure is trustworthy – SQL code written in a stored procedure can be trusted. But…
Code within a web application is trustworthy – SQL code written in a web application can be trusted. But…
…but all this can be undermined if your developers tell user-entered code to be executed, rather than being treated as parameters.
(Of course, if your server is compromised and someone can tell rogue commands to run, then all bets are off – I’m just looking at SQL Injection here)
What happens with SQL Injection is this: a COMMAND is constructed that includes user-entered DATA, and is then executed. It doesn’t matter whether this is done within .NET code, or within perl code, or within a stored procedure – you just don’t ever execute data.
There are plenty of guides to avoiding SQL Injection. They say to avoid using dynamic SQL; to avoid ad hoc SQL; to only use stored procedures. Most of them miss the point. They’re not bad, but following these rules doesn’t make you injection-proof. You simply don’t ever execute anything that came from the user.
Dynamic SQL is fine. You can construct a query based on user input – so long as you don’t execute something that came from the user. If the user chooses a particular option, the dynamically-generated SQL can include or exclude some predicate, or can bring a new parameter into the mix. This gives the freedom of dynamic SQL without needing to become vulnerable to SQL Injection.
Ad hoc SQL is also fine, and you don’t need to use stored procedures. It doesn’t matter whether your SQL code is stored in a stored procedure within the database, or whether your SQL code is stored within your application.
Ad hoc code or dynamic SQL like this is okay:
|
1 2 3 4 5 |
if (UserInputtedFirstName.Length > 0) { cmd += " AND u.FirstName = @fn "; params.Add("@fn", ....... ...... } |
but code like this is not:
|
1 2 3 4 |
if (UserInputtedFirstName.Length > 0) { cmd += " AND u.FirstName = " + Sanitise(UserInputtedFirstName); ...... } |
…no matter what the Sanitise functionality is. The only way to protect against SQL Injection is to only use controlled commands, and never anything else.
But what about generic procedures, that developers use to be able to be able to query whichever objects they want?
It’s easy to simply “Just say no”, to procedures like this. After all – it’s not healthy for all kinds of reasons, such as the plan cache, such as the effective use of indexes, and more. But if you have a stored procedure like:
|
1 2 3 4 5 |
create proc dbo.GetAnyData(@tablename nvarchar(256), @colname nvarchar(256), @filtercol nvarchar(256), @filterval sql_variant) as begin declare @qry nvarchar(max) = 'select ' + @colname + ' from ' + @tablename + ' where ' + @filtercol + ' = @val'; exec sp_executesql @qry, '@val sql_variant', @val = @filterval; end |
…with an application that uses it, then you may be somewhat worried (and legitimately so) about SQL Injection, and be wondering how you’re going refactor all the code that uses this procedure.
So how do you make this safe from SQL Injection, so that the calls to the procedure aren’t susceptible, while all the redevelopment is done?
Hopefully you know the answer – you need to control the commands.
The issue is not with the use of @filterval – that’s being passed in as data, and is being used as data. It’s never part of the command itself. The problem is that @tablename, @colname and @filtercol are all passed in from the calling application, potentially even from the user, and then used as part of the command. This is what needs to be fixed.
Luckily, we have access to a bunch of things that we know can be used here – actual table names and actual column names in the database’s catalog views. So therefore, we can safely use those things. They are controlled.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
create proc dbo.GetAnyData(@tablename nvarchar(256), @colname nvarchar(256), @filtercol nvarchar(256), @filterval sql_variant) as begin declare @knowntable nvarchar(256), @knowncol nvarchar(256), @knownfiltercol nvarchar(256); select @knowntable = quotename(object_name(object_id)), @knowncol = quotename(name) from sys.all_columns where object_id = object_id(@tablename) and name = @colname; select @knownfiltercol = quotename(name) from sys.all_columns where object_id = object_id(@tablename) and name = @filtercol; declare @qry nvarchar(max) = 'select ' + @knowncol + ' from ' + @knowntable + ' where ' + @knownfiltercol + ' = @val'; if @qry is not null exec sp_executesql @qry, N'@val sql_variant', @val = @filterval; end |
I’m not saying this is good. I’m saying this is SAFE. We don’t execute anything that we don’t already control. The only tables we can query are ones that we know actually exist. The only columns that we can use are ones that must exist. You can always add more logic, to make sure that people can’t query system objects, and things like that – but they definitely can’t execute anything overly malicious. They can’t make calls to sp_configure or to xp_cmdshell. They can’t drop tables or update values. If you need to restrict the columns, you can easily filter the queries on all_columns, so that the calling code can’t access so much.
SQL Injection is serious. It’s a huge problem for people who deal with databases – and not just SQL Server, but all databases. The solution is easy, but requires some level of discipline.
Don’t execute user input, and don’t let your developers do it either (and if that means forcing them to use stored procedures that you can audit, then fine – do whatever it takes).
Oh, and as this month’s T-SQL Tuesday (hosted by Kenneth Fisher – @sqlstudent144) is on the topic of security, that’s what this post is on.
This Post Has 6 Comments
Excellent post, thanks for this! I’ll be sharing this page liberally 🙂
Succinct Quote to explain SQL Injection.
Thank you.
Found a couple of typos in your code that prevent it from executing:
"where object_id = object_id(@tablename) and name = @fitercol; "
should be @filtercol (mising the "l")
and I believe you’ll need an "N" in the sp_executesql statement for the parameter @val, as sp_executesql requires Unicode parameters.
Found a typo in your “typo” reply. Mising should be missing.
Ah yes. Thanks Rich. 🙂
Pingback: Em desenvolvimento