How to drop SQL table from C# without opening to code injection?

Viewed 52

Edit: I marked that the linked question was useful, but I didn't intend to mean that it completely answered my question here, which after reviewing others' comments and answers here, I realize I need two different escaped versions of table, one as a safe single-quoted identifier, and one as a safe single-bracketed identifier.


I need to drop a SQL server table from C# with the table name as a parameter.

This is my code:

private static void DropSqlTableIfExists(string connectionString, string table)
{
    string query = "if object_id (@table, 'U') is not null begin drop table @table; end";
    using SqlConnection conn = new(connectionString);
    using SqlCommand command = new(query, conn);
    command.Parameters.AddWithValue("@table", table);
    conn.Open();
    command.ExecuteNonQuery();
}

The above code gives me an error, Incorrect syntax near '@table'.

I could of course put the table name directly into the query string, but that would allow code injection, which I need to avoid.

What is the best way to go about this?


EDIT 2:

Based on feedback, I updated the code to look like this:

private static void DropSqlTableIfExists(string connectionString, string table)
{
    string tableSafeQuoted = $"'{table.Replace("'", "''")}'";
    string tableSafeBracketed = $"[{table.Replace("]", "]]").Replace(".", "].[")}]";

    string query = $"if object_id ({tableSafeQuoted}, 'U') is not null begin drop table {tableSafeBracketed}; end";
    using SqlConnection conn = new(connectionString);
    using SqlCommand command = new(query, conn);
    conn.Open();
    command.ExecuteNonQuery();
}

tbh, I'm not sure if it's 100% safe, due to the oddity of the . replacement.. but I think unless someone gives me a good reason not to, I will leave it like this..

2 Answers

You'll need to escape/encode the table name as an identifier. In SQL Server, this is typically done by replacing any closing brackets (]) in the name with two consecutive brackets (]]), and then surrounding the result with brackets ([...]).

You can either write a simple method to do this yourself, or take on a dependency like ScriptDom, which allows you to do this:

var escapedTableName = Identifier.EncodeIdentifier(tableName);

If your SQL Server database is set to use double-quoted identifiers, do something similar, but with double-quotes instead of brackets. Or, with ScriptDom:

var escapedTableName = Identifier.EncodeIdentifier(tableName, QuoteType.DoubleQuote);

Note that encoding the table name makes any .s act as if they're part of the table name itself. If your table name is supposed to have qualifiers (database, schema), each of those must be escaped individually, so you'll probably want to have them passed in as a separate argument, or create a separate type to represent the combination of these names.

There is probably a more elegant way to do this, but here's one possible way:

declare @dynamicsql nvarchar(max)
set @dynamicsql = 'DROP TABLE dbo.' + QUOTENAME(@table) + '';
if object_id (@table, 'U') is not null
begin
    EXEC sp_executesql @dynamicsql;
end

so in your specific case it will be:

private static void DropSqlTableIfExists(string connectionString, string table)
{
    string query = @"
        declare @dynamicsql nvarchar(max)
        set @dynamicsql = 'DROP TABLE dbo.' + QUOTENAME(@table) + '';
        if object_id (@table, 'U') is not null
        begin
            EXEC sp_executesql @dynamicsql;
        end
    ";
    using SqlConnection conn = new(connectionString);
    using SqlCommand command = new(query, conn);
    command.Parameters.AddWithValue("@table", table);
    conn.Open();
    command.ExecuteNonQuery();
}

I hope it helps!

Related