Sql-server – Should we still be using QUOTENAME to protect from injection attacks

sql-injectionsql-server-2008t-sql

I was looking at an old stored procedure today and noticed it was using quotename on the input parameters. After doing some digging to figure out what that does exactly I came across this site. I now understand what it does and how to use it but the site says it is used as a mitigation from SQL Injection attacks. When I used to develop apps that directly queried a database, using asp.net, I would use ADO.Net parameters to pass user input in as a literal value and never really worried about protecting it in my stored procedures.

I am now writing a stored procedure that will be used by applications that I do not write so I do need to try and protect from injection attacks at the procedure level, is the quotename the best way to do this or is there a newer function/better method?

Code that got me on this thought pattern (@parm1 is a user input parameter):

'SELECT project [Project], project_desc [Description], 
        customer [Customer], cpnyid [Company]
FROM PJPROJ (nolock)
where project like ' + quotename(@parm1,'''') + '

Best Answer

Yes, things haven't changed much in this area, you should be using quotename for any SQL server object names that are used in dynamic SQL (especially if they are supplied externally to your code). As well as SQL injection mitigation this also means your code will work correctly for non standard identifier names.

The function is only appropriate for object names (e.g. table, column, database names) though.

You should try and parameterise everything else and use sp_executesql, passing in parameters rather than concatenating those into the query string.

The definitive article on this topic is still The Curse and Blessings of Dynamic SQL

Edit. Now you have supplied the code I see it is passing the second parameter of ' to add the outer quotes and escape any single quotes by doubling them up before injecting them into the string. This is not a good use of quotename. It will fail (return null) if the string is greater than 128 characters.

Additionally It may still leave SQL injection possibilities if the string contains U+02BC instead of the standard apostrophe and then the string is assigned to a varchar after the sanitation (where it can silently get converted to a regular apostrophe )

The correct way to do this is to leave the query parameterised. And then pass the @parm1 value in to sys.sp_executesql

DECLARE @Sql NVARCHAR(MAX);

SET @Sql = '
SELECT project      [Project],
       project_desc [Description],
       customer     [Customer],
       cpnyid       [Company]
FROM   PJPROJ (nolock)
WHERE  project LIKE @parm1 
';

EXEC sys.sp_executesql
  @Sql,
  N'@parm1 varchar(100)',
  @parm1 = 'foobar%';
Related Question