SQL Server - How to protect against SQL Injection when dealing with a Dynamic WHERE clause passed to a Stored Procedure

Question

Given the following stored procedure, can I protect against SQL injection in the supplied additional WHERE condition?

CREATE PROCEDURE [dbo].[ProtectDynamicWhereClause]
   (@TableName varchar(50),
    @OldestRecordDate varchar(15),
    @WhereCondition varchar(250) = NULL)
AS
BEGIN
    -- Protect the table name from SQL Injection.
    DECLARE @TableNameClean varchar(150) = QUOTENAME(@TableName)

    DECLARE @sql nvarchar(4000)
    SET @sql = 
            N'
            SELECT * FROM ' + 'dbo.' + @TableNameClean
            + ' WHERE EntryAge > DATEDIFF(year, @OldestRecordDate, GETDATE()) '

    IF (@WhereCondition IS NOT NULL)
    BEGIN
        SET @sql = @sql + ' AND ' + @WhereCondition
    END               

    EXEC  sp_executesql @sql, N'@TableNameClean varchar(50), @OldestRecordDate varchar(15)', 
                                @TableNameClean = @TableNameClean, @OldestRecordDate = @OldestRecordDate
END

EXEC ProtectDynamicWhereClause @TableName='CustomerTbl', @OldestRecordDate = '2012-01-01', @WhereCondition = 'CustomerName LIKE ''%Smith%'''
EXEC ProtectDynamicWhereClause @TableName='ProductTbl', @OldestRecordDate = '2010-01-01', @WhereCondition = 'ProductId IN (123, 345, 567)'

Could you please tell me how to protect against SQL injection in this statement when the @WhereCondition is actually passed-into the stored procedure as a parameter as shown?

SET @sql = @sql + ' AND ' + @WhereCondition

Thank you.

Answer 1

You simply don't write it in such a way that a user can pass in a structured WHERE clause. This is a recipe for disaster, and I bet at least half of the companies who have been exploited by SQL injection thought they were protected by checking the input for keywords, stripping out semi-colons and comments, etc. They weren't, and there will always be ways around whatever protection you try to manually write.

Don't be lazy. Write it so that the user picks the possible columns and operations from drop-downs, and only enters the parameter values into free text. Then you construct the dynamic SQL with the columns you know exist and with strongly-typed parameters that can only be treated as such rather than just appending your query with whatever someone typed into a form field on a web page and blindly executing it.

Surely there aren't 8,000 columns in this table, and the number of permutations of clauses that a user can add are not exhaustive. You could always limit it logically to 5 or 10 clauses so that you don't have GB-long WHERE clauses.

Some constructive ideas for dealing with dynamic SQL and flexible WHERE clauses:

• Protecting Yourself from SQL Injection - Part 1
• Protecting Yourself from SQL Injection - Part 2

I have videos about my solution to "the kitchen sink" here and here as well as a blog post about it.

Other answers where I talk about the kitchen sink procedure:

• Tsql Query speed slow due to Or within where clause, causing index scan instead of seek
• Plan cache memory: parameterized SQL vs stored procedures
• SQL Server procedure optimisation
• Strange looking where clause in sql server
• Stored procedure with recompile

Answer 2

The only way to protect against injection here would be to write a parser, in or called from this stored procedure, for the SQL language parts that you wish to allow - that code could validate the strings to make sure they do nothing nasty. This almost certainly isn't practical nor would it be remotely efficient written in TSQL - so technically the answer is yes there is a way, but practically the answer is no.

If you only want to allow a very small subset of the SQL language - i.e. all the clauses will be of the form <fieldname> LIKE '%<string>%' or <fieldname> IN (number, number, number) parsing the string to make sure it matches those would be more practical, but this would be better done in your layer above in your application where it'll be more efficient. You might have to include a fixed list of possible column names as the user that the application is running as may not be able to select from sys.columns to verify the provided names that way.

Answer 3

I have the same problem with a read mostly database. It contains the metadata from a machine learning algorithm that classifieds images. The main search form grew too large to use the GET method, which people liked because they could copy the URI and include it in an email or a slack post.

The easiest way to give them such a link was too include the where clause. Since I'm working with Java servlets and I control the generation of the where clause it was fairly straightforward to produce a validator that is too simple to call a parser. Here is the method that I call. It will take some work to adapt to your problem but I think it's easy enough to understand how it works;

  /**
 * Verify a where clause is valid
 * example:  GPStime >= 1164554240 AND GPStime <= 1188081664  AND  ifo = 'L1' 
 *          AND  pipeline = 'GravitySpy' AND  label = 'Blip' AND  
 *          confidence  >= 0.960 AND confidence <= 1.00 
 *          ORDER BY confidence DESC, snr DESC
 * Note all tokens must be separated by whitespace, I'm lazy
 * @param where - clause to be validated
 * @return - True if we like the clause, Full survey it fails
 */
public boolean checkWhere(String where)
{
    boolean ret = true;
    Pattern[] pats = 
    {
        Pattern.compile("^'.*'$"),  // quoted string
        Pattern.compile("^[\\<\\>\\=]{1,2}$"),  // Number comparator
        // acceptable keyword
        Pattern.compile("(?i)^(and|or|like|order|by|ASC|DESC),{0,1}$"),
        // floating-point number
        Pattern.compile("^\\s*[+-]?\\s*((\\d+\\.?\\d*)|(\\.\\d+))\\s*["
                + "Ee]?\\s*([+-]?\\d+)?\\s*$")
    };
    Pattern identifier = Pattern.compile("[\\w_]+"); // column name

    StringTokenizer toks = new StringTokenizer(where);

    // loop verifies that each token is acceptable ignoring the order
    while (toks.hasMoreTokens())
    {
        String token = toks.nextToken();
        Matcher m;
        boolean ok = false;
        for (Pattern pat : pats)
        {
            m = pat.matcher(token);
            if (m.find())
            {
                ok = true;
                break;
            }
        }
        if (!ok)
        {
            m = identifier.matcher(token);
            if (m.find())
            {
                if (!fieldNames.containsKey(token))
                {
                    ret = false;
                    break;
                }
            }
            else
            {
                ret = false;
                break;
            }
        }
    }

    return ret;
}