Correct escaping of delimited identifers in SQL Server without using QUOTENAME

Is there anything else that the code must do to sanitize identifiers (table, view, column) other than to wrap them in double quotation marks and "double up" double quotation marks present in the identifier name? References would be appreciated.

I have inherited a code base that has a custom object-relational mapping (ORM) system. SQL cannot be written in the application but the ORM must still eventually generate the SQL to send to the SQL Server. All identifiers are quoted with double quotation marks.

string QuoteName(string identifier) 
{ 
    return "\"" + identifier.Replace("\"", "\"\"") + "\"";
}

If I were building this dynamic SQL in SQL, I would use the built-in SQL Server QUOTENAME function:

declare @identifier nvarchar(128);
set @identifier = N'Client"; DROP TABLE [dbo].Client; --';

declare @delimitedIdentifier nvarchar(258);
set @delimitedIdentifier = QUOTENAME(@identifier, '"');

print @delimitedIdentifier;
-- "Client""; DROP TABLE [dbo].Client; --"

I have not found any definitive documentation about how to escape quoted identifiers in SQL Serve开发者_开发问答r. I have found Delimited Identifiers (Database Engine) and I also saw this stackoverflow question about sanitizing.

If it were to have to call the QUOTENAME function just to quote the identifiers that is a lot of traffic to SQL Server that should not be needed.

The ORM seems to be pretty well thought out with regards to SQL Injection. It is in C# and predates the nHibernate port and Entity Framework etc. All user input is sent using ADO.NET SqlParameter objects, it is just the identifier names that I am concerned about in this question. This needs to work on SQL Server 2005 and 2008.

---

Update on 2010-03-31

While the application is not supposed to allow user-input for identifier names in queries, the ORM does via the query syntax that it has for both ORM-style reads and custom queries. It is the ORM that I am trying to ultimately prevent all possible SQL Injection attacks as that is very small and easy to verify as opposed to all the application code.

A simple example of the query interface:

session.Query(new TableReference("Client")
    .Restrict(new FieldReference("city") == "Springfield")
    .DropAllBut(new FieldReference("first_name"));

ADO.NET sends over this query:

exec sp_executesql N'SELECT "T1"."first_name" 
FROM "dbo"."Client" AS "T1" 
WHERE "T1"."city" = @p1;', 
N'@p1 nvarchar(30)', 
N'Springfield';

Perhaps it would help to think about how something similar this might look in nHibernate Query Language (HQL):

using (ISession session = NHibernateHelper.OpenSession())
{
    Client client = session
        .CreateCriteria(typeof(Client))  \\ <-- TableReference in example above
        .Add(Restrictions.Eq("city", "Springfield"))  \\ <-- FieldReference above
        .UniqueResult<Client>();
    return client;
}

Maybe I should look and see how nHibernate protects the input.

---

Your QuoteName function needs to check the length, because the T-SQL QUOTENAME function specifies the maximum length it returns. Using your example:

String.Format(@"declare @delimitedIdentifier nvarchar(258);
set @delimitedIdentifier = {0};", QuoteName(identifier));

If QuoteName(identifier) is longer than 258 characters, it will be silently truncated when assigned to @delimitedIdentifier. When that happens, you open up the possibility for @delimitedIdentifier to be escaped improperly.

There is an MSDN article by Bala Neerumalla, a "security software developer at Microsoft", that explains the topic in more depth. The article also contains the closest thing I have found to "definitive documentation about how to escape quoted identifiers in SQL Server":

The escaping mechanism is simply doubling up the occurrences of right square brackets. You don't need to do anything with other characters, including left square brackets.

This is the C# code I am currently using:

/// <summary>
/// Returns a string with the delimiters added to make the input string
/// a valid SQL Server delimited identifier. Brackets are used as the
/// delimiter. Unlike the T-SQL version, an ArgumentException is thrown
/// instead of returning a null for invalid arguments.
/// </summary>
/// <param name="name">sysname, limited to 128 characters.</param>
/// <returns>An escaped identifier, no longer than 258 characters.</returns>
public static string QuoteName(string name) { return QuoteName(name, '['); }

/// <summary>
/// Returns a string with the delimiters added to make the input string
/// a valid SQL Server delimited identifier. Unlike the T-SQL version,
/// an ArgumentException is thrown instead of returning a null for
/// invalid arguments.
/// </summary>
/// <param name="name">sysname, limited to 128 characters.</param>
/// <param name="quoteCharacter">Can be a single quotation mark ( ' ), a
/// left or right bracket ( [] ), or a double quotation mark ( " ).</param>
/// <returns>An escaped identifier, no longer than 258 characters.</returns>
public static string QuoteName(string name, char quoteCharacter) {
    name = name ?? String.Empty;
    const int sysnameLength = 128;
    if (name.Length > sysnameLength) {
        throw new ArgumentException(String.Format(
            "name is longer than {0} characters", sysnameLength));
    }
    switch (quoteCharacter) {
        case '\'':
            return String.Format("'{0}'", name.Replace("'", "''"));
        case '"':
            return String.Format("\"{0}\"", name.Replace("\"", "\"\""));
        case '[':
        case ']':
            return String.Format("[{0}]", name.Replace("]", "]]"));
        default:
            throw new ArgumentException(
                "quoteCharacter must be one of: ', \", [, or ]");
    }
}

---

Can you not just use [ and ] delimiters instead of quotes (single or double)?

Identifiers should never really contain any quotes (unless you're more unlucky than now) so you remove the normal use factor of quotes in names etc

Edit:

But if the calls to the ORM are already parameterised, you don't need to worry about it, no? Using [ and ] removes the need for complex escaping in c# strings