c# - How to avoid SQL Injection when using '[' and ']' characters for schema/table names?

2
votes

I have had several contexts where table names or schemas were not hard-coded, but rather configured by the administrator, or, worse, generated from user input.

Since cases were easy (schemas and table names in plain English, without numbers nor symbols), it was easy to avoid SQL Injection by just forbid any character outside A-Z and a-z ranges. But this approach sucks when an application must handle any Unicode characters when can be a part of a name of a schema or a table.

Now, if an SQL query uses '[' and ']' to include names to schema, table and column, is it enough to forbid only ']' character in names to avoid SQL Injection?

Example:

A'B is a valid name for a table. So:

string tableNameFromUserInput = "A'B"; // Let's imagine it is a user input.
using (SqlCommand getEverything = new SqlCommand(
    string.Format("select * from [dbo].[{0}]", tableNameFromUserInput),
    sqlConnection)
{
    // Do stuff.
}

is perfectly valid, and there is no reason to block the single quote character.

So is there a risk of SQL Injection in the following code?

string tableName = UserInput.GetUnsafeInputFromUser();

// Search for ']' character only.
if (tableName.IndexOf(']') != -1)
{
    throw new HackerDetectedException();
}
else
{
    using (SqlCommand getEverything = new SqlCommand(
        string.Format("select * from [dbo].[{0}]", tableNameFromUserInput),
        sqlConnection)
    {
        // Do stuff.
    }
}

Edit:

After some search, what I found is an article on Delimited identifiers in Microsoft SQL. According to it:

The body of the identifier can contain any combination of characters in the current code page, except the delimiting characters themselves.

By the way, the delimited identifiers are limited to 128 characters.

So to make it work:

  • The ']' character must be forbidden (or, better, replaced by ']]'; see Mark Byers answer below).
  • The length must be limited to 128 characters (not done in the sample code above),
  • The code page must probably be checked to get the correct table.
4
c#securitytsqlsql-injection
Table names generated from user input? Wow, what's the scenario where that would make sense? – Tomalak
@Tomalak: I agree, it doesn't make too much sense most of the times. If you want an example, for schemas, it would be the possibility for an administrator to change the schema for an application before deployment (in some circumstances, administrators must not be trusted). Another example for table names: where an application enables to do stuff on several similar tables, and where new tables can be created but must not require additional work, nor the need to recompile the whole application. – Arseni Mourzenko
I'm sorry, these reasons are too weak for me. "Administrators must not be trusted"? Hm, they have admin rights, if they must do stupid things there's nothing you can do to prevent them. "do stuff on several similar tables" does not mean creating new tables is the right thing to do. To the contrary, it's a hint that there is a nasty design error in the app. – Tomalak
@Tomalak: "Hm, they have admin rights, if they must do stupid things there's nothing you can do to prevent them" If you are an administrator of your website hosted on a shared service, you may want to customize the names of a table or a schema of your website, but it would be too bad to give you a complete trust on the whole system. "it's a hint that there is a nasty design error in the app." I agree. – Arseni Mourzenko

4 Answers

3
votes

I think that would be safe. Characters such as backslash are treated as literal characters so they should be safe to include in table names. The only character I can immediately think of as being a problem is ], which you already disallow. By the way, to include ] in a table name it needs to be written as ]], so instead of denying it completely you could instead do tableName.Replace("]", "]]").

However to be on the safe side you should use a whitelist as you suggested. To do this while also allowing for unicode characters in table names you might want to consider using the regular expression @"^\w+$" to validate the table name as this accepts unicode characters too.

2
votes

if it is only select sql i.e. you are not intended to save any changes,you can run your sql command in a transaction in a try block and rollback in finally block.

this way , you can ensure at your code level that any other ddl , dml would not be run from your code. only select satements will be executing.

This particualr startegey works when you fetching data but not updating.

   try
        {
            //Run command in transaction.
        }
        catch (Exception ex)
        {
            // rollback transaction
        }

        finally
        {
            // rollback transaction
        }

and also this has been tested for Sybase database

2
votes

Not sure entirely how complicated your inline SQL is, but why not just pass the schema/table strings from the user through QUOTENAME() to escape invalid characters?

0
votes

I think the following would crush your database perhaps the following from OWASP:
'and 1 in (select min(name ) from master.dbo.sysdatabases where name >'.' ) --'

For anyone who wants the OWASP resources:
OWASP