T-SQL Dynamic pivot query and SQL Injection protection-CodePudding

When importing/parsing flat files and delimited data (we use SqlBulkCopy for these) we end up with a table with a DATA_KEY and DATA_VAL column. [EDIT] Forgot to mention this table is a work table for the .net crl to parse the various inbound data only. To load the data into the production tables from this work table we then use a dynamic query to PIVOT the data like so:

DECLARE @sqlCommand varchar(MAX)
DECLARE @columnList varchar(max)

SELECT 
    @columnList = STUFF((SELECT DISTINCT',MIN(CASE DATA_KEY WHEN ''' DATA_KEY ''' THEN DATA_VAL END) AS '  QUOTENAME(DATA_KEY) 
                         FROM PARSED_DATA
                         FOR XML PATH ('')), 1, 1, '')

SET @sqlCommand = 'SELECT ROW_NUMBER() OVER(ORDER BY KEYID,PARENT_POS) AS ROWID,  '   @columnList   ' FROM PARSED_DATA

This works fine, but brings about the question of possible SQL injection. Obviously this is a concatenated string, and so DATA_KEY and DATA_VAL could potentially be subject to injection. When I look at the DATA_KEY field, it seems that the only way injection could work is if the attacker knew this was inside a CASE statement- that is, the injected code would have to END the CASE statement properly or the entire query would fail. Similarly, the DATA_VAL field would fail the entire query if the injection did not properly END the CASE statement.

Am I correct here? Or am I missing something obvious? Also, where DATA_KEY is used as the column name are we susceptible there? QUOTENAME make the value a valid SQL identifier; is it possible to have injection there?

Just looking to make sure all bases are covered; comments and suggestions welcome!

CodePudding user response：

There are a few issues I can see:

You should use QUOTENAME on the pivot CASE expression also. Use '''' as the second parameter for this, to get '' as the delimiter
FOR XML should use .value to unescape XML characters
The third parameter of STUFF is the length. To ensure it is always correct, use a @separator variable
Dynamic SQL should be in nvarchar(max)
The performance of DISTINCT over a calculate field may be slow, it may be wiser to group first in a derived table/subquery

DECLARE @separator nvarchar(10) = ',';

DECLARE @columnList nvarchar(max) = STUFF(
    (SELECT @separator   'MIN(CASE DATA_KEY WHEN '   QUOTENAME(DATA_KEY, '''')   ' THEN DATA_VAL END) AS '   QUOTENAME(DATA_KEY)
     FROM (SELECT DISTINCT DATA_KEY FROM PARSED_DATA) pd
     FOR XML PATH (''), TYPE
).value('text()[1]','nvarchar(max)'), 1, LEN(@separator), '');

You say you are using this to generate an INSERT statement, so you should also check the column names against sys.columns