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'