How to Sanitize TSQL

Question

I always use SqlParameter to parametrize my queries when calling SQL Server from C#, nevertheless, some of my queries generate an ugly execution plan when using Parameters, in these cases, I rather use hard-coded SQL instead of parameters. Is this a good practice? - maybe not, but performance of the query is great!

I have to use this technique because of Parameter Sniffing in SQL Server.

Now, do anyone has a library to sanitize TSQL code in order to avoid SQL injection? - if not, what should I check to prevent this? (maybe I will end-up creating a NuGet package).

Maybe you know other way to avoid this problem that allows me to keep using SqlParameters?

Thanks!

UPDATE 1: This question comes up because of this other one Improve performance of query with conditional filtering here you can see why I had to come to this solution.

UPDATE 2: In order to complete the context of the question: I have the following query, it is called from C# code using ADO.NET, when using parameters, the query takes from 7s to several minutes to execute depending on the arguments.

SELECT SKU, Store, ColumnA, ColumnB, ColumnC
FROM myTable
WHERE (SKU IN (select * from splitString(@skus)) OR @skus IS NULL)
AND (Store IN (select * from splitString(@stores)) OR @stores IS NULL)

@skus and @stores are strings with concatenated ids like '1,2,3' and splitString is a database function which transforms that string in a table of 1 column and each item in a row, allowing me to execute IN filtering in SQL Server.

When executing this very same query without parameters and concatenating directly in the query the ids, it takes between 0s and 5s to execute depending on the ids. I am aware about performace issues when using a function inside the where clause, and I treated that in the final form of the query, but it does not impact on the performance as much as using raw sql. Please read this and take the idea and not literally.

Since I'm being forced to use the second approach because of the performance, how can I avoid SQL Injection?

Answer 1

Like I wrote in the comments (that was moved to chat) - Use table valued parameters instead of SQL comma-delimited string split. And since I can't close as duplicate I might as well give an answer here.

Using a table valued parameter requires a stored procedure and a user defined table type, so let's go ahead and create the table type:

CREATE TYPE dbo.IntList AS TABLE
(
    val int NOT NULL PRIMARY KEY
);
GO

Now let's create the stored procedure:

CREATE PROCEDURE dbo.GetValuesBySkuOrStore
(
    @skus dbo.IntList readonly,
    @stores dbo.IntList readonly,
)
AS

    SELECT SKU, Store, ColumnA, ColumnB, ColumnC
    FROM myTable
    LEFT JOIN @skus skus ON SKU = skus.val
    LEFT JOIN @stores stores ON Store = stores.val
    WHERE skus.val IS NOT NULL 
    OR stores.val IS NOT NULL 

GO

Now, to execute this from c#, you need to first create a DataTable for each table valued parameter, and then add them to the parameters collection with the type SqlDbType.Structured. Something like this:

DataTable CreateIntList(IEnumerable<int> values)
{
    var dt = new DataTable();
    // Note: Columns order matters! 
    // if you have a udt with multiple columns, 
    // your data table must reflect the columns exactly in the same order.
    dt.Columns.Add("val", typeof(int));
    foreach(var val in values)
    {
        dt.Rows.Add(val);
    }
}

void GetValuesBySkuOrStore(Ienumerable<int> skus, Ienumerable<int> stores)
{
    var skusDT = CreateIntList(skus);
    var storesDT = CreateIntList(stores); 

    using(var con = new SqlConnection(connectionString))
    {
        using(var cmd = new SqlCommand("GetValuesBySkuOrStore", con))
        {
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.Add("@skus", SqlDbType.Structured).Value = skus;
            cmd.Parameters.Add("@stores", SqlDbType.Structured).Value = stores;
            con.Open();
            using(var reader = cmd.ExecuteReader())
            {
                while(reader.Read())
                {
                    // do your stuff here...
                }
            }
        }
    }
}

This might not have the same performance as hard-coding the values but it is protected from SQL Injection and will have better performance over splitting comma-separated values in SQL Server.

Answer 2

Someone may prove me wrong but if sku and store are integers then there is no SQL injection

where sku in (1, 2, 3) is safe

I the string parses down to integers you are good
If it does not parse down to integers the don't let it through

If you really want to use a function then call it once and store the output in a table value parameter (TVP)