how to return result of select while trying a safe generic stored procedure using sp_executesql

Question

I was checking a proper way for querying database via using parameterized command and still be able to pull a query that will match any table as a generic SP. This sql sp seems without potentially opening up a hole for SQL injection and do correct me if I am wrong this is safe... But here the question is, using `sp_executesql` (seems to be the key element for being safe) does not return the SELECT results. How could I alter that stored procedure to return the value (and not 'ruining' its safety) ``` CREATE PROCEDURE [dbo].[SafeSqlSP_SelectGivenTableWithOptionalColFilter] @columnList varchar(75) ='*', @tableName sysname , @ColNameAsFilter1 varchar(75) ='', @ColNameAsFilter2 varchar(75) ='', @ColFilter1VAL varchar(75)='', @ColFilter2VAL varchar(75)='' AS BEGIN SET NOCOUNT ON; DECLARE @sqlCommand nvarchar(1000) if( @ColNameAsFilter2!='' AND @ColNameAsFilter1!='') begin SET @sqlCommand = 'SELECT ' + QUOTENAME(@columnList) + ' FROM ' + QUOTENAME(@tableName) +' WHERE ' + QUOTENAME(@ColNameA…

Accepted Answer

Here is what I have so far. With the exception of columnNames (which I'll get to in a sec) I've tested each of the columns that is not paramaterized using sp_executesql.

I'm not entirely pleased with the way I ended up testing columnNames but I don't think this is truly a problem. First of all I would expect columnList to be coming in from an application, not a user. A user should not know enough about the structure of your database to be filling in the first 7 parameters and a developer has far more risks that SQL Injection. That being said the only SQL injection I could think of that would work given where columnList is placed would have to contain the word "FROM" with a space on either side of it. So I tested for that and raised an error if it occurs. This would be a problem if you happen to have a column named "This column is FROM somewhere else", but let's hope you don't.

Just a note. I used the `sys.all_objects` and `sys.all_columns` system views for my tests in case system tables were queried by the procedure. If you change them to the more commonly used `sys.objects` and `sys.columns` the stored procedure will not work for system views.

EDIT: I added a test on columnList to check for a semicolon. I realized something along the lines of `'''abc''; print ''test''; select * '` would be a problem.

EDIT2: I've updated the procedure to handle the column list better. Thanks @HABO!. I've also added code to handle if values were pre `QUOTENAME`ed with []s, "s or 's, and some common formats for * in the column list. I'm using the splitter function that @HABO has listed, so I'm not listing it here.

```
ALTER PROCEDURE [dbo].[SafeSqlSP_SelectGivenTableWithOptionalColFilter] 
    @columnList nvarchar(max) ='*',
    @tableSchema sysname ,
    @tableName sysname ,
    @ColNameAsFilter1 nvarchar(255) ='',
    @ColNameAsFilter2 nvarchar(255) ='',
    @ColFilter1VAL nvarchar(max)='',
    @ColFilter2VAL nvarchar(max)=''       
    AS

BEGIN
    SET NOCOUNT ON;

--====================================================
    -- Set default values
    IF ISNULL(@tableSchema,'') = ''
        SET @tableSchema = 'dbo'
    ELSE
        SET @tableSchema = LTRIM(RTRIM(@tableSchema))

IF ISNULL(@columnList,'') = ''
        SET @columnList = '*'

SET @tableName = ISNULL(LTRIM(RTRIM(@tableName)),'')
    SET @ColNameAsFilter1 = ISNULL(LTRIM(RTRIM(@ColNameAsFilter1)),'')
    SET @ColNameAsFilter2 = ISNULL(LTRIM(RTRIM(@ColNameAsFilter2)),'')
    SET @ColFilter1VAL = ISNULL(@ColFilter1VAL,'')
    SET @ColFilter2VAL = ISNULL(@ColFilter2VAL,'')

--====================================================
    -- Remove probably QUOTENAMEs from @tableSchema and @tableName before testing them
    SET @tableSchema = CASE WHEN LEFT(@tableSchema,1) = '[' AND RIGHT(@tableSchema,1) = ']'
                            THEN SUBSTRING(REPLACE(@tableSchema,']]',']'),2,LEN(REPLACE(@tableSchema,']]',']'))-2)
                        WHEN LEFT(@tableSchema,1) = '"' AND RIGHT(@tableSchema,1) = '"'
                            THEN SUBSTRING(REPLACE(@tableSchema,'""','"'),2,LEN(REPLACE(@tableSchema,'""','"'))-2)
                        WHEN LEFT(@tableSchema,1) = '''' AND RIGHT(@tableSchema,1) = ''''
                            THEN SUBSTRING(REPLACE(@tableSchema,'''''',''''),2,LEN(REPLACE(@tableSchema,'''''',''''))-2)
                        ELSE @tableSchema END

SET @tableName = CASE WHEN LEFT(@tableName,1) = '[' AND RIGHT(@tableName,1) = ']'
                            THEN SUBSTRING(REPLACE(@tableName,']]',']'),2,LEN(REPLACE(@tableName,']]',']'))-2)
                        WHEN LEFT(@tableName,1) = '"' AND RIGHT(@tableName,1) = '"'
                            THEN SUBSTRING(REPLACE(@tableName,'""','"'),2,LEN(REPLACE(@tableName,'""','"'))-2)
                        WHEN LEFT(@tableName,1) = '''' AND RIGHT(@tableName,1) = ''''
                            THEN SUBSTRING(REPLACE(@tableName,'''''',''''),2,LEN(REPLACE(@tableName,'''''',''''))-2)
                        ELSE @tableName END

--====================================================
    -- Test to make sure the schema.table exists
    IF NOT EXISTS (
                    SELECT 1 
                    FROM sys.all_objects
                    JOIN sys.schemas
                        ON sys.all_objects.schema_id = sys.schemas.schema_id
                    WHERE sys.all_objects.name = @tableName
                      AND sys.schemas.name = @tableSchema
                      AND sys.all_objects.[TYPE] IN ('S','U','V')
                    )
        BEGIN
            RAISERROR (N'Table %s.%s does not exist.',
                        16,
                        1,
                        @tableSchema,
                        @tableName)
            RETURN
        END

--====================================================
    -- Test to make sure all of the comma delimited values 
    -- are valid columns for schema.table

-- Create and populate a list of columns from columnli…

how to return result of select while trying a safe generic stored procedure using sp_executesql

Solution

Problem