The important thing to note in the query above is that we are generating a dynamic SQL statement; that is, we are building the SQL query string, and then we are executing it.

Imagine this stored procedure is running in order to display a “Welcome <Full Name>!” message in our app — a website visitor types in their@ParmUserName and we execute the stored procedure to return their full name.

Here’s our code that calls the stored procedure:

EXEC dbo.sp_GetFullName 'TFly37'

And result:

Cool. No problems so far.

However, what if our user decides to pass in the following value as their username?

EXEC dbo.sp_GetFullName 'TFly37'' or 1=1 --'

This funny looking parameter value returns this:

AHHHHHH!!!!

This user just hacked our website and viewed all of the users in our table.

In this specific case only our user’s full names were breached, but in other instances it’s just as easy for more sensitive data like passwords, social security numbers, and bank account numbers to be revealed as well (If you are looking for some fun, search for “SQL injection” on the Have I been pwned? list of Pwned websites to see all of the companies that aren’t doing a good job protecting your privacy).

So how did that example of SQL injection actually work?

Since our stored procedure executes a dynamically generated query, let’s look at what that generated query actually looks like for both of the parameters that were passed in:

Even though TFly37'' or 1=1-- doesn’t look like a intelligible input parameter, when its concatenated into our query it makes sense.

Our malicious user is basically writing their own SQL query, one that will return all of the names of our users instead of just their own. In many instances, the crafty hacker can go a step further and write additional injection code to reveal the contents of the entire user table (or other tables in the database)!

Are any of my current SQL queries vulnerable to SQL injection attacks?

So SQL injection is really bad and you don’t want to become like Sony or Yahoo. How do we check to see if we have any vulnerable queries on our server?

I wrote the query below to help you start searching. It is not perfect, but it does act as a good starting point for auditing potentially injectable queries:

-- This file tries to find stored procedures and functions that *may* be vulnerable to SQL injection attacks.
-- It works by searching your database for occurences of "+" signs followed by "@", indicating that SQL parameters
-- might be getting concatenated to a dynamic SQL string. It also checks for the existence of 'EXEC' to see if any
-- strings are being executed.
-- Not every result returned will be susceptible to SQL injection, however they should all be examined to see if they are vulnerable.
-- Originally fromn: https://github.com/bertwagner/SQLServer/blob/master/SQL%20Injection%20Vulnerabilities.sql
SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED
SELECT
ROUTINE_CATALOG,
ROUTINE_SCHEMA,
ROUTINE_NAME,
ROUTINE_TYPE,
ROUTINE_DEFINITION
FROM
INFORMATION_SCHEMA.ROUTINES
WHERE
REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(ROUTINE_DEFINITION,CHAR(0),''),CHAR(9),''),CHAR(10),''),CHAR(11),''),CHAR(12),''),CHAR(13),''),CHAR(14),''),CHAR(160),''),' ','')
LIKE '%+@%'
AND
( -- Only if executes a dynamic string
ROUTINE_DEFINITION LIKE '%EXEC(%'
OR ROUTINE_DEFINITION LIKE '%EXECUTE%'
OR ROUTINE_DEFINITION LIKE '%sp_executesql%'
)

I’ve found this script useful for myself, however if you find any issues with it please let me know, thanks!

Looking for a script to find non-SARGable queries on your server? Scroll to the bottom of this post.

What is a “SARGable” query?

Just because you add an index to your table doesn’t mean you get immediate performance improvement. A query running against that table needs to be written in such a way that it actually takes advantage of that index.

SARGable, or “Search Argument-able”, queries therefore are queries that are capable of utilizing indexes.

SELECT Name
FROM dbo.CoffeeInventory
WHERE CONVERT(CHAR(10),CreateDate,121) = '2017-08-19'

Although this query correctly filters our rows to a specific date, it does so with this lousy execution plan:

SQL Server has to perform an Index Scan, or in other words has to check every single page of this index, to find our ‘2017–08–19’ date value.

SQL Server does this because it can’t immediately look at the value in the index and see if it is equal to the ‘2017–08–19’ date we supplied — we told it first to convert every value in our column/index to a CHAR(10) date string so that it can be compared as a string.

Since the SQL Server has to first convert every single date in our column/index to a CHAR(10) string, that means it ends up having to read every single page of our index to do so.

The better option here would be to leave the column/index value as a datetime2 datatype and instead convert the right hand of the operand to a datetime2:

SELECT Name
FROM dbo.CoffeeInventory
WHERE CreateDate = CAST('2017-08-19' AS datetime2)

In this scenario SQL gives us an Index Seek because it doesn’t have to modify any values in the column/index in order to be able to compare it to the datetime2 value that ‘2017–08–19’ got converted to.

This means SQL only has to read what it needs to output to the results. Much more efficient.

One more example

Based on the last example we can assume that any function, explicit or implicit, that is running on the column side of an operator will result in a query that cannot make use of index seeks, making it non-SARGable.

In short, keep in mind whether SQL Server will have to modify the data in a column/index in order to compare it — if it does, your query probably isn’t SARGable and you are going to end up scanning instead of seeking.

OK, non-SARGable queries are bad…how do I check if I have any on my server?

The script below looks at cached query plans and searches them for any table or index scans. Next, it looks for scalar operators, and if it finds any it means we have ourselves a non-SARGable query. The fix is then to rewrite the query to be SARGable or add a missing index.

-- From https://github.com/bertwagner/SQLServer/blob/master/Non-SARGable%20Execution%20Plans.sql
-- This script will check the execution plan cache for any queries that are non-SARGable.
-- It does this by finding table and index scans that contain a scalar operators
SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED
DECLARE @dbname SYSNAME
SET @dbname = QUOTENAME(DB_NAME());
WITH XMLNAMESPACES (DEFAULT 'http://schemas.microsoft.com/sqlserver/2004/07/showplan')
SELECT
stmt.value('(@StatementText)[1]', 'varchar(max)') AS [Query],
query_plan AS [QueryPlan],
sc.value('(.//Identifier/ColumnReference/@Schema)[1]', 'varchar(128)') AS [Schema],
sc.value('(.//Identifier/ColumnReference/@Table)[1]', 'varchar(128)') AS [Table],
sc.value('(.//Identifier/ColumnReference/@Column)[1]', 'varchar(128)') AS [Column] ,
CASE WHEN s.exist('.//TableScan') = 1 THEN 'TableScan' ELSE 'IndexScan' END AS [ScanType],
sc.value('(@ScalarString)[1]', 'varchar(128)') AS [ScalarString]
FROM
sys.dm_exec_cached_plans AS cp
CROSS APPLY sys.dm_exec_query_plan(cp.plan_handle) AS qp
CROSS APPLY query_plan.nodes('/ShowPlanXML/BatchSequence/Batch/Statements/StmtSimple') AS batch(stmt)
CROSS APPLY stmt.nodes('.//RelOp[TableScan or IndexScan]') AS scan(s)
CROSS APPLY s.nodes('.//ScalarOperator') AS scalar(sc)
WHERE
s.exist('.//ScalarOperator[@ScalarString]!=""') = 1
AND sc.exist('.//Identifier/ColumnReference[@Database=sql:variable("@dbname")][@Schema!="[sys]"]') = 1
AND sc.value('(@ScalarString)[1]', 'varchar(128)') IS NOT NULL

I’ve found this script useful for myself, but if you find any issues with it please let me know, thanks!