Could SQL Injection be dangerous even when perfectly safe?

SQL Injection is something I would expect any reader here to be familiar with. Despite being one of the oldest database attack methods, it still persists for decades on the OWASP Top Ten list of critical security risks to web applications.

In fact, instead of dying out, it only seems to be getting more clever and even automated. With "hacker bots" scouring the web and automatically probing for injection vulnerabilities to exploit. I know, as I've once been a victim of such attacks in the past.

But today I'm not actually going to talk about that. Today, I'm going to ask the question: When is SQL Injection dangerous, even if it's perfectly safe?

What does it even mean a "perfectly safe SQL Injection"?

That's a perfectly valid question, dear reader.

For example, let's say that we have a table called "Accounts" which contains, among other things, a numeric "ID" column and a textual "Username" column.

Let's assume that we have something like the following stored procedure:

CREATE PROCEDURE dbo.GetAccountDetails
	@Username nvarchar(100)
AS

DECLARE @AccountID int, @Command nvarchar(MAX);

SELECT @AccountID = ID
FROM dbo.Accounts
WHERE Username = @Username

IF @AccountID IS NOT NULL
BEGIN
	SET @Command = N'
	SELECT FullName, Email, OtherStuff
	FROM dbo.Accounts
	WHERE ID = ' + CAST(@AccountID AS nvarchar(MAX))

	EXEC(@Command)
END
ELSE
BEGIN
	RAISERROR(N'Username "%s" was not found',16,1,@Username);
END

Note a few things here:

• The only parameter received from the end-user is @Username.

• This @Username is used for retrieving an @AccountID which is an integer data type.

• There is SQL Injection happening in lines 13-18 involving the @Command variable, but it's only using the integer variable @AccountID, which by definition cannot contain malicious data that "breaks" the dynamic SQL command.

• Therefore, there is no risk here of a malicious attack from an end-user.

Another example of a perfectly safe SQL injection could be something that dynamically affects which tables and columns you'd be querying from. For example:

CREATE PROCEDURE dbo.AggregateTransactions
	@AggFunction sysname = 'COUNT',
	@DataMode tinyint = 1
AS
DECLARE @CMD nvarchar(MAX)

SET @CMD = N'
SELECT SomeColumn, AggValue = ' +
CASE WHEN @AggFunction IN ('SUM', 'MIN', 'MAX', 'COUNT', 'AVG')
	THEN @AggFunction
	ELSE 'COUNT'
	END
	+ N'('
	+ CASE @DataMode WHEN 1 THEN 'MoneyAmount' WHEN 2 THEN 'TaxAmount' WHEN 3 THEN 'DiscountAmount' ELSE 'MoneyAmount' END
	+ N')
FROM dbo.SomeTable
GROUP BY SomeColumn'

EXEC sp_executesql @CMD

Note how both procedure parameters @AggFunction and @DataMode are never concatenated into the dynamic SQL command without first being validated against an explicit set of values.

As another example, I could also mention one of my older blog posts about Dynamic Search Queries in SQL Server, which implements a method that on the one hand uses dynamic SQL and SQL injection, but on the other hand, it actually protects from malicious SQL injections, as none of the injections within it come from the end-user.

In general, a "perfectly safe SQL Injection" is when the "injection" is not actually utilizing any input that comes from end-users. Instead, the injection is using static data, code-driven data, or some kind of metadata queried directly from a table that doesn't require validation as its content doesn't come from end-users.

So, when is a "safe" injection no longer "safe"?

A while back I had a customer with a very interesting issue.

They complained about performance problems but couldn't tell what was the root cause of it.

I connected to their SQL Server machine and started collecting some monitoring data to get an idea of what was going on.

What I saw was several executions of a query that took a very long time to execute... But didn't actually have high resource consumption counters (i.e. CPU, logical reads, writes, network throughput, etc.). These queries weren't blocked by any other session either.

So, what was happening?

I saw the problem immediately once I opened up the full command text of the query.

What I saw was a fairly simple query, with a couple of INNER JOINs, a few WHERE predicates, and...

one huge...

ENORMOUS...

HUMONGOUS...

... STUPENDOUSLY long list of values separated by a comma and used within an IN predicate.

and on and on it went

Let me tell you, friend... That list of IDs went on for days!

In fact, it was so long, that the query text size was measured in tens of kilobytes!

After some investigation, we found that this list of IDs was a "perfectly safe SQL injection" of numerical IDs stored in the application's cache. This list did not come from any end-user input, but it didn't exist in any table in the database either.

Microsoft Docs has this to say about the IN operator:

Explicitly including an extremely large number of values (many thousands of values separated by commas) within the parentheses, in an IN clause can consume resources and return errors 8623 or 8632. To work around this problem, store the items in the IN list in a table, and use a SELECT subquery within an IN clause.

Error 8623: The query processor ran out of internal resources and could not produce a query plan. This is a rare event and only expected for extremely complex queries or queries that reference a very large number of tables or partitions. Please simplify the query. If you believe you have received this message in error, contact Customer Support Services for more information.

Error 8632: Internal error: An expression services limit has been reached. Please look for potentially complex expressions in your query, and try to simplify them. source: IN (Transact-SQL) | Microsoft Docs

While in this particular use case, the list of values wasn't long enough to produce errors 8623 or 8632, it was still long enough to cause some seriously heavy burden on the SQL Optimizer trying to compile this query, and thus slow the duration of the query so much - all simply because of its compile time.

There are a few alternatives to using long lists of explicit values for an IN operator, such as querying from a database table, a temporary table, a table variable, or a table-valued parameter.

For this particular use case, the customer didn't have these values stored anywhere inside the database, as they were calculated by the application during runtime.

So, the best alternative I could suggest to them was to send a table-valued parameter to a stored procedure, instead of dynamically constructing the ad-hoc command.

ahh... peace at last

However, had it been possible, I would've rather suggested using a purely data-driven solution by querying the list of values from a (persisted) table in the database.

Conclusion

As a bottom line, my message to you is this:

SQL injection can indeed be "safe" sometimes when done correctly.

But you must also be careful not to go too crazy with it, as you may inadvertently generate a query that is so long and complex, that it essentially breaks down the SQL engine during the compilation phase.