Stored procedure to query custom data tables as dynamic SQL

Question

There's a lot going on here, but the background for why this is necessary is that there is a set schema, or 'core' set of tables that are prefixed with 'bu', and any core table can have a custom table of the same base name but prefixed with xb and has the same primary Key. I have no idea if a customer has created an extension table, but assume they know its structure and the data model.

This sproc is called via a web API endpoint that the customer will manage, so there may be cases where it is an unsecured call. Does this open the door to destructive injection attacks? Also, there is some duplicated functionality that could be moved into a function or CTE, but I ran out of time and SQL prowess.

SET QUOTED_IDENTIFIER OFF
GO
SET ANSI_NULLS OFF
GO
/* *********************************************************************************
-- CHANGE LOG
-- *********************************************************************************
-- Date Changed Who Desc/Comments
-- ------------ -------- ----------------------------------------------------------
-- 12/10/2015 This Guy Created - Gets any Core data by Xtend table name
 and specially formatted where clause. 
 Parameters: 
 @tableName is the name of the xtend
 table. It must begin with 'xb' or the sproc 
 returns 0.
 @returnColumns is a comma delimited list of columns
 in the Core (bu) table that are returned. '*' can 
 be passed in to retrieve all the columns in the 
 Core table (None of the columns in the joined tables will be returned).
 @whereClauses (@xbWhereClause - required, @coreWhereClause -optional)
 are comma delimited where clauses in the
 following format: {column}:{operator}:{Value}
 All clauses are treated as 'AND' clauses.
 Example: 
 Special_Request_Created:>:1/1/2013,Special_Request_Created:<:1/1/2016
 This evaluates to 
 WHERE 
 ( 
 xb.Special_Request_Created > '1/1/2013'
 AND
 xb.Special_Request_Created < '1/1/2016'
 )
 SPECIAL CASE
 When the Operator is 'IN' the user needs to wrap
 the clause with parens () and quote string data.
 Also, the comma (,) needs to be designated as [c]
 Special_Request_ID:IN:(3456[c]6789)
 Latest_Special_Status:IN:(''New''[c]''Final'')
 @joinClause is a comma delimited list of table joins in the following format:
 {column in core table:coreTable2:column}. 
 Example:
 Foo_Detail_Line_ID:buFoo_Detail_Line:Foo_Detail_Line_ID,
 Foo_ID:buFoo_Header:Foo_ID
 This evaluates to
 JOIN buFoo_Detail_Line bu2
 ON bu.Foo_Detail_Line_ID = bu2.Auth_Detail_Line_ID
 JOIN buFoo_Header bu3
 ON bu2.Foo_ID = bu3.Foo_ID
 This script returns data in the following format: 
 Id Key Value
 Table PK Column Name Column Value
-- *********************************************************************************/
CREATE PROCEDURE [API].[GetCoreFieldsByXtendColumnValues]
(
 @tableName varchar(128),
 @returnColumns varchar(max),
 @xbWhereClause nvarchar(max),
 @coreWhereClause nvarchar(max) = '',
 @joinClause nvarchar(max) = ''
)
AS
BEGIN
 SET NOCOUNT ON;
 DECLARE @SQL nvarchar(max) = '',
 @columns nvarchar(max),
 @conversion nvarchar(max),
 @pk nvarchar(max);
 DECLARE @coreTableName varchar(128) = 'bu' + RIGHT(@tableName,LEN(@tableName)-2)
 IF((SELECT LEFT(@tableName,2)) = 'xb')
 BEGIN
 DECLARE @columnTable TABLE 
 (
 COLUMN_NAME nvarchar(max)
 )
 --Get Columns and Verify they exist in the table
 IF (@returnColumns = '*')
 BEGIN
 INSERT INTO @columnTable
 SELECT 'bu.' + c.COLUMN_NAME
 FROM INFORMATION_SCHEMA.COLUMNS c
 WHERE c.TABLE_NAME = @coreTableName
 INSERT INTO @columnTable
 SELECT 'xb.' + c.COLUMN_NAME
 FROM INFORMATION_SCHEMA.COLUMNS c
 WHERE c.TABLE_NAME = @tableName
 AND NOT EXISTS (SELECT 1 FROM @columnTable WHERE COLUMN_NAME = 'bu.'+c.COLUMN_NAME)
 END
 ELSE
 BEGIN
 INSERT INTO @columnTable
 SELECT 'bu.' + c.COLUMN_NAME
 FROM dbo.AESplit(@returnColumns) r
 JOIN INFORMATION_SCHEMA.COLUMNS c
 ON r.ArrayColumn = c.COLUMN_NAME
 WHERE c.TABLE_NAME = @coreTableName
 INSERT INTO @columnTable
 SELECT 'xb.' + c.COLUMN_NAME
 FROM dbo.AESplit(@returnColumns) r
 JOIN INFORMATION_SCHEMA.COLUMNS c
 ON r.ArrayColumn = c.COLUMN_NAME
 WHERE c.TABLE_NAME = @tableName
 AND NOT EXISTS (SELECT 1 FROM @columnTable WHERE COLUMN_NAME = 'bu.'+c.COLUMN_NAME)
 END
 DECLARE @pkTable table(
 DB varchar(255), 
 [SCHEMA] varchar(255), 
 [TABLE] varchar(255), 
 [COLUMN] varchar(max), 
 [KEY_SEQ] int, 
 [PK_NAME] nvarchar(255) 
 );
 INSERT INTO @pkTable
 EXEC sp_pkeys @tableName
 SET @pk = (SELECT TOP 1 [COLUMN] FROM @pkTable)
 SELECT @columns = (SELECT STUFF((Select ','+ RIGHT(C.COLUMN_NAME,LEN(C.COLUMN_NAME)-3)
 FROM @columnTable C
 FOR XML PATH('')),1,1,''))
 SELECT @conversion = (SELECT STUFF((SELECT ', CONVERT(VARCHAR(MAX), '+ C.COLUMN_NAME + ',120) AS ' + RIGHT(C.COLUMN_NAME,LEN(C.COLUMN_NAME)-3)
 FROM @columnTable C
 FOR XML PATH('')),1,1,''))
 --Set up Where Clauses TODO: This should be a CTE to avoid duplication 
 DECLARE @whereTable1 table
 (
 Id int Primary Key,
 Value nvarchar(max)
 )
 DECLARE @whereTable2 table
 (
 Id int,
 IndexNum int,
 Value nvarchar(max)
 )
 DECLARE @formattedXbWhere nvarchar(max) = '',
 @formattedCoreWhere nvarchar(max) = '',
 @formattedJoin nvarchar(max) = ''
 --xbWhereClause
 BEGIN
 INSERT INTO @whereTable1 (Id, Value)
 SELECT t.IndexNum, t.Data
 FROM dbo.SplitString(@xbWhereClause,',') t
 INSERT INTO @whereTable2
 SELECT t.Id, v.IndexNum, v.Data
 FROM @whereTable1 t
 CROSS APPLY dbo.SplitString(t.Value,':') v
 --SELECT * from @whereTable2
 SELECT @formattedXbWhere += N'WHERE (' + STUFF((Select ' AND '
 + 'xb.' + pvt.[1] + ' ' 
 + pvt.[2] + ' ' 
 + CASE 
 WHEN pvt.[2] = 'IN' THEN pvt.[3]
 WHEN (ISNUMERIC(pvt.[3]) = 0) THEN '''' + pvt.[3] + '''' 
 ELSE pvt.[3] 
 END
 FROM (
 SELECT Id, IndexNum, Value
 FROM @whereTable2
 ) as p
 PIVOT
 (
 MAX(Value)
 FOR IndexNum IN ([1],[2],[3])
 ) AS pvt
 FOR XML PATH('')),1,5,'') + ' )'
 SET @formattedXbWhere = REPLACE(REPLACE(REPLACE(@formattedXbWhere,'&gt;','>'),'&lt;','<'),'[c]',',')
 --SELECT @formattedXbWhere
 END
 --coreWhereClause
 IF (@coreWhereClause <> '')
 BEGIN
 PRINT 'Core WHERE Exists';
 DELETE FROM @whereTable1
 DELETE FROM @whereTable2
 INSERT INTO @whereTable1 (Id, Value)
 SELECT t.IndexNum, t.Data
 FROM dbo.SplitString(@coreWhereClause,',') t
 INSERT INTO @whereTable2
 SELECT t.Id, v.IndexNum, v.Data
 FROM @whereTable1 t
 CROSS APPLY dbo.SplitString(t.Value,':') v
 --SELECT * from @whereTable2
 SELECT @formattedCoreWhere += N' AND (' + STUFF((Select ' AND '
 + CASE
 WHEN (SELECT COUNT(1) FROM @columnTable WHERE COLUMN_NAME = pvt.[1]) = 1 THEN 'bu.' + pvt.[1] + ' '
 ELSE pvt.[1] + ' '
 END
 --+'bu.' + pvt.[1] + ' ' 
 + pvt.[2] + ' ' 
 + CASE 
 WHEN pvt.[2] = 'IN' THEN pvt.[3]
 WHEN (ISNUMERIC(pvt.[3]) = 0) THEN '''' + pvt.[3] + '''' 
 ELSE pvt.[3] 
 END
 FROM (
 SELECT Id, IndexNum, Value
 FROM @whereTable2
 ) as p
 PIVOT
 (
 MAX(Value)
 FOR IndexNum IN ([1],[2],[3])
 ) AS pvt
 FOR XML PATH('')),1,5,'') + ' )'
 SET @formattedCoreWhere = REPLACE(REPLACE(REPLACE(@formattedCoreWhere,'&gt;','>'),'&lt;','<'),'[c]',',')
 --SELECT @formattedCoreWhere;
 END
 --Set Up Join Clause
 IF (@joinClause <> '')
 BEGIN 
 PRINT 'JOIN Exists';
 DELETE FROM @whereTable1
 DELETE FROM @whereTable2 
 INSERT INTO @whereTable1 (Id, Value)
 SELECT t.IndexNum, t.Data
 FROM dbo.SplitString(@joinClause,',') t
 INSERT INTO @whereTable2
 SELECT t.Id, v.IndexNum, v.Data
 FROM @whereTable1 t
 CROSS APPLY dbo.SplitString(t.Value,':') v
 --SELECT * from @whereTable2
 SELECT @formattedJoin += 'JOIN ' + STUFF((Select 'JOIN ' +
 + pvt.[2] + ' bu' + CONVERT(varchar(10),Id) + ' ON ' +
 CASE WHEN Id = 1 THEN ' bu.'
 ELSE ' bu' + CONVERT(varchar(10),Id-1) + '.' END 
 + pvt.[1] + ' = bu' + CONVERT(varchar(10),Id) + '.' 
 + pvt.[3] + ' ' 
 FROM (
 SELECT Id, IndexNum, Value
 FROM @whereTable2
 ) as p
 PIVOT
 (
 MAX(Value)
 FOR IndexNum IN ([1],[2],[3])
 ) AS pvt
 FOR XML PATH('')),1,4,'')
 END
 SELECT @SQL += N'SELECT DISTINCT ' + @pk + '2 AS Id, [Key], [Value] FROM (SELECT bu.' + @pk + ' AS ' + @pk + '2, ' + @conversion + 
 ' FROM ' + @coreTableName + ' bu JOIN ' + @tableName + ' xb ON bu.' + @pk + ' = xb.' + @pk
 + ' ' + @formattedJoin
 + ' ' + @formattedXbWhere + 
 CASE
 WHEN @formattedCoreWhere <> '' THEN @formattedCoreWhere
 ELSE ''
 END
 + ') x ' --+' WHERE ' + @columnName + ' = ' + @columnValue +') x '
 + 'UNPIVOT ( [Value] FOR [Key] IN (' + @columns + ')) AS unpiv'
 --SELECT @SQL AS query;
 EXEC sp_executesql @SQL
 END
 ELSE RETURN 0
END
GO
/*
--Example:
EXEC [API].[GetCoreFieldsByXtendColumnValues] 'xbSpecial_Request', '*','Latest_Special_Status:=:New', 'Vendor_ID:=:123456','Foo_Detail_Line_ID:buFoo_Detail_Line:Foo_Detail_Line_ID,Foo_ID:buFoo_Header:Auth_ID'
*/

Answer 1

Does this open the door to destructive injection attacks?

It depends, but potentially, yes it can, especially with dynamic SQL, which you should try to avoid unless it is absolutely necessary. You should be able to sanitize the input before passing it to the database server, how to do it really depends on what technologies you are using.

NOTE: DO NOT TRY THIS ON A PRODUCTION DATABASE

What would happen if the stored procedure was called with @xbWhereClause = 'Vendor_ID:=:123456; DELETE FROM xbSpecial_Request;--'? If you are lucky it will throw a syntax error. But the SQL procedure is not "smart" to know whether it should execute arbitrary code or not, it just does it usually. Just keep that in mind and make sure to sanitize database inputs before it even reaches the database server.

---

Another disadvantage of dynamic SQL is that it can be quite slow, as SQL engine has to calculate a new execution plan whenever you call EXEC sp_executesql @SQL. It is also more prone to errors due to the amount of string manipulation that is needed to make it work, which in your case is quite a lot.

---

PRINT statements

PRINT statements don't really belong in production SQL code. They are slow and often not very useful (how often do you go check the SQL Server console log for statements like "Core WHERE Exists"?)

If you need to log this information somewhere, log it either in a table, or some external file system that is searchable.

---

INFORMATION_SCHEMA

Here is an article about: The case against INFORMATION_SCHEMA views by DBA extraordinaire Aaron Bertrand. For the reasons cited in the article, it is better (more informative) to query the sys schema rather than INFORMATION_SCHEMA.

For example, this statement:

 SELECT 'bu.' + c.COLUMN_NAME
 FROM INFORMATION_SCHEMA.COLUMNS c
 WHERE c.TABLE_NAME = @coreTableName

Could be rewritten as such:

 SELECT 'bu.' + c.name
 FROM sys.columns AS c
 JOIN sys.tables AS t on c.object_id = t.object_id
 WHERE t.name = @coreTableName

If you need to you can also join sys.schemas to sys.tables on their common schema_id key, in case you need to use the schema names for something.

---

Too much

I think you should really re-evaluate if this is a good idea. Perhaps have a DBA at your organization look over the logic to point out how it could be improved.

Just looking at the way this is called, it really looks like there is something problematic with this complicated "string builder" that finally executes a dynamic SQL statement... Most SQL stored procedure calls aren't supposed to look like that, and I think if you're going to do this you may as well have the calling application build the query instead of having a database procedure for it.

EXEC [API].[GetCoreFieldsByXtendColumnValues] 'xbSpecial_Request', '*','Latest_Special_Status:=:New', 'Vendor_ID:=:123456','Foo_Detail_Line_ID:buFoo_Detail_Line:Foo_Detail_Line_ID,Foo_ID:buFoo_Header:Auth_ID'