SQLServerCentral Article

Why QUOTENAME is important

,

At some point, the vast majority of us are going to need to use dynamic SQL in our career. Dynamic SQL isn’t a bad thing, but making sure it is safe is really important. One of the simplest tools to do so is good use of QUOTENAME.

Dealing with dynamic objects

Let’s take a simple stored procedure like the below, where someone has tried to make it “safe”:

  CREATE PROC CreateTable_sp @TableName sysname AS
  DECLARE @DSQL nvarchar(MAX);
  SET @DSQL = N'CREATE TABLE [' + @TableName + N'] (ID int)';
  EXEC (@DSQL);
  GO

On the face of it you can see they’ve tried to avoid injection, however it is far from safe. Like with concatenating a set of strings enclosed in single quotes, the same injection techniques can be used here.

  EXEC dbo.CreateTable_sp @TableName = 'asdklfhjgasdklf] (ID int); SELECT 1 AS test;--';

This would easily get around the parameter being encapsulated, and would create a nonsense named table as well as return a dataset with the column test. If we implement the use of QUOTENAME, however, then we’re in a much better place:

  CREATE PROC CreateTable_sp @TableName sysname AS
  DECLARE @DSQL nvarchar(MAX);
  SET @DSQL = N'CREATE TABLE ' + QUOTENAME(@TableName) + N' (ID int);';
  EXEC sp_executesql @DSQL;
  GO

If we were to run the same SQL we did above, then you would actually end up with a table with the name “asdklfhjgasdklf] (ID int); SELECT 1 AS test;--“.

QUOTENAME, however, shouldn’t just be considered when you’re creating an object like above. Dynamic Pivots are pretty common, and QUOTENAME‘s use is just as important there. You may feel you’re safer against injection, as the column names are more known. That doesn’t, however, mean someone hasn’t been “silly” enough to use a special character in a column’s name. If you have a column with a name like CustomerID[int] then (like before) the syntax '[' + [name] + N']' isn’t going to work. It’s not going to inject anything with that name, but will cause an error.

When the dynamic item isn’t an object

Although I’m repeating what the documentation (linked above) tells you, I think it’s important to note that QUOTENAME actually accepts 2 parameters. The first is the “character string” and the second (optional) parameter is the “quote character”. According to the documentation the definition of the quote character Is a one-character string to use as the delimiter. Can be a single quotation mark ( ' ), a left or right bracket ( [] ), or a double quotation mark ( " ). If quote_character is not specified, brackets are used.” However I have found that other characters are accepted. For example both ( and { work and I’m sure there are more. If a character isn’t acceptable then NULL will be returned; for example QUOTENAME('SomeString','#') will return NULL.

When dealing with dynamic SQL, you may not always be dealing with a dynamic object. For example, you might be using dynamic SQL with OPENROWSET. When declaring the Filepath for your file, it needs to be quoted in a single quote ('), not brackets ([]). This means QUOTENAME(@FilePath) isn’t going to work, but change that to QUOTENAME(@FilePath,''''), and it will.

If we take a (not so) simple stored procedure:

  CREATE PROC ImportData_sp @NewTable sysname, @ImportFile nvarchar(512), @Worksheet nvarchar(128) AS
  DECLARE @DSQL nvarchar(MAX);
  SET @DSQL = N'SELECT ''' + @Worksheet + ''' AS Worksheet, *' + NCHAR(10) +
  N'INTO [' + @NewTable + N']' + NCHAR(10) +
  N'FROM OPENROWSET(''Microsoft.ACE.OLEDB.12.0'',' + NCHAR(10) +
  N' ''Excel 12.0; Database=' + REPLACE(@ImportFile,N'''',N'') + ',' + NCHAR(10) +
  N' [''' + @Worksheet + N'$'']);';
  EXEC (@DSQL);
  GO

Unlike before, we have a few dynamic items; the destination object, the source file and the worksheet in that file. This actually leaves a few areas open, which is far from ideal. But we can still fix using similar methodology from before:

  CREATE PROC ImportData_sp @NewTable sysname, @ImportFile nvarchar(512), @Worksheet nvarchar(128) AS
  DECLARE @DSQL nvarchar(MAX);
  SET @DSQL = N'SELECT @dWorkSheet AS WorkSheet, *' + NCHAR(10) +
  N'INTO ' + QUOTENAME(@NewTable) + + NCHAR(10) +
  N'FROM OPENROWSET(''Microsoft.ACE.OLEDB.12.0'',' + NCHAR(10) +
  N' ' + QUOTENAME('Excel 12.0; Database=' + @ImportFile,'''') + ',' + NCHAR(10) +
  N' ' + QUOTENAME(QUOTENAME(@Worksheet + '$','''')) + N');';
  --PRINT @DSQL;
  EXEC sp_executesql @DSQL, N'@dWorkSheet nvarchar(128)', @dWorksheet = @Worksheet;
  GO

There’s a bit more to take in here (for example, note the nested QUOTENAME for the Worksheet), this makes the SQL far safer than it was. A user can easily execute it like the below, and provided they don’t have single quotes, or other bad special characters, in the worksheet’s name (try as I might I couldn’t get that to work, if someone knows how, please comment) then it’ll successfully import:

  EXEC dbo.ImportData_sp @NewTable = 'JanData',
  @ImportFile = 'C:\My Files\ExcelDoc.xlsx;',
  @Worksheet = 'Jan data';

Conclusion

QUOTENAME seems to, sometimes, be a “forgotten” function. When working with Dynamic SQL, raw string concatenation is always a bad idea. Making sure you correctly parametrise your query, and quoting the string you can’t, however, considerably lowers the exposure you have to injection.

Rate

4.54 (26)

You rated this post out of 5. Change rating

Share

Share

Rate

4.54 (26)

You rated this post out of 5. Change rating