Menu
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(@ColNameAsFilter1) +' = @ColFilter1VAL AND ' + QUOTENAME(@ColNameAsFilter2) +' = @ColFilter2VAL'
EXECUTE sp_executesql @sqlCommand,
N'@ColFilter1VAL nvarchar(75), @ColFilter2VAL nvarchar(75)', @ColFilter1VAL= @ColFilter1VAL, @ColFilter2VAL = @ColFilter2VAL
end
else if( @ColNameAsFilter1!='' AND @ColNameAsFilter2='')
begin
SET @sqlCommand = 'SELECT ' + QUOTENAME(@columnList) + ' FROM ' + QUOTENAME(@tableName) +' WHERE ' + QUOTENAME(@ColNameAsFilter1) +' = @ColFilter1VAL'
EXECUTE sp_executesql @sqlCommand,
N'@ColFilter1VAL nvarchar(75)', @ColFilter1VAL= @ColFilter1VAL
end
else if( @ColNameAsFilter1='' AND @ColNameAsFilter2='')
begin
SET @sqlCommand = 'SELECT ' + QUOTENAME(@columnList) + ' FROM ' + QUOTENAME(@tableName)
EXECUTE sp_executesql @sqlCommand
end
END
332
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 QUOTENAMEed 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 columnlist
DECLARE @ColumnListTable TABLE (Item varchar(255))
INSERT INTO @ColumnListTable
SELECT Item
FROM dbo.SplitCSL(@columnList)
-- Remove any extra spaces
UPDATE @ColumnListTable SET Item = LTRIM(RTRIM(Item))
-- "Fix" any * formats to a single format of [schema].[tablename].*
UPDATE @ColumnListTable SET Item = CASE WHEN Item IN (
@tableName + '.*', @tableName + '.[*]',
'[' + @tableName + '].*', '[' + @tableName + '].[*]',
@tableSchema + '.' + @tableName + '.*', @tableSchema + '.' + @tableName + '.[*]',
@tableSchema + '.' + '[' + @tableName + '].*', @tableSchema + '.' + '[' + @tableName + '].[*]',
'[' + @tableSchema + '].' + @tableName + '.*', '[' + @tableSchema + '].' + @tableName + '.[*]',
'[' + @tableSchema + '].' + '[' + @tableName + '].*', '[' + @tableSchema + '].' + '[' + @tableName + '].[*]'
)
THEN '[' + @tableSchema + '].' + '[' + @tableName + '].*'
WHEN Item IN ('*','[*]') THEN '*'
ELSE Item END
--====================================================
-- Remove probably QUOTENAMEs from columns in column list before testing them
UPDATE @ColumnListTable SET Item =
CASE WHEN LEFT(Item,1) = '[' AND RIGHT(Item,1) = ']'
THEN SUBSTRING(REPLACE(Item,']]',']'),2,LEN(REPLACE(Item,']]',']'))-2)
WHEN LEFT(Item,1) = '"' AND RIGHT(Item,1) = '"'
THEN SUBSTRING(REPLACE(Item,'""','"'),2,LEN(REPLACE(Item,'""','"'))-2)
WHEN LEFT(Item,1) = '''' AND RIGHT(Item,1) = ''''
THEN SUBSTRING(REPLACE(Item,'''''',''''),2,LEN(REPLACE(Item,'''''',''''))-2)
ELSE Item END
-- Check for invalid column names
DECLARE @ColumnListFailures AS varchar(max)
SET @ColumnListFailures = ''
SELECT @ColumnListFailures = STUFF((
SELECT ', ' + Item
FROM @ColumnListTable
WHERE Item NOT IN (SELECT name
FROM sys.all_columns
WHERE object_id = OBJECT_ID(@tableSchema+'.'+@tableName))
AND Item <> '[' + @tableSchema + '].' + '[' + @tableName + '].*'
FOR XML PATH(''),TYPE).value('.','VARCHAR(MAX)')
,1,2, '')
IF LEN(@ColumnListFailures) > 0
BEGIN
RAISERROR (N'Table %s.%s does not have columns %s that are listed in the columnList parameter.',
16,
1,
@tableSchema,
@tableName,
@ColumnListFailures)
RETURN
END
-- QUOTENAME each of the column names and re-create @ColumnList
SELECT @ColumnList = STUFF((
SELECT ', ' + CASE WHEN Item = '[' + @tableSchema + '].' + '[' + @tableName + '].*' THEN Item
ELSE QUOTENAME(Item) END
FROM @ColumnListTable
FOR XML PATH(''),TYPE).value('.','VARCHAR(MAX)')
,1,2, '')
--====================================================
-- Remove probably QUOTENAMEs from first and second column filters before testing them
SET @ColNameAsFilter1 = CASE WHEN LEFT(@ColNameAsFilter1,1) = '[' AND RIGHT(@ColNameAsFilter1,1) = ']'
THEN SUBSTRING(REPLACE(@ColNameAsFilter1,']]',']'),2,LEN(REPLACE(@ColNameAsFilter1,']]',']'))-2)
WHEN LEFT(@ColNameAsFilter1,1) = '"' AND RIGHT(@ColNameAsFilter1,1) = '"'
THEN SUBSTRING(REPLACE(@ColNameAsFilter1,'""','"'),2,LEN(REPLACE(@ColNameAsFilter1,'""','"'))-2)
WHEN LEFT(@ColNameAsFilter1,1) = '''' AND RIGHT(@ColNameAsFilter1,1) = ''''
THEN SUBSTRING(REPLACE(@ColNameAsFilter1,'''''',''''),2,LEN(REPLACE(@ColNameAsFilter1,'''''',''''))-2)
ELSE @ColNameAsFilter1 END
SET @ColNameAsFilter2 = CASE WHEN LEFT(@ColNameAsFilter2,1) = '[' AND RIGHT(@ColNameAsFilter2,1) = ']'
THEN SUBSTRING(REPLACE(@ColNameAsFilter2,']]',']'),2,LEN(REPLACE(@ColNameAsFilter2,']]',']'))-2)
WHEN LEFT(@ColNameAsFilter2,1) = '"' AND RIGHT(@ColNameAsFilter2,1) = '"'
THEN SUBSTRING(REPLACE(@ColNameAsFilter2,'""','"'),2,LEN(REPLACE(@ColNameAsFilter2,'""','"'))-2)
WHEN LEFT(@ColNameAsFilter2,1) = '''' AND RIGHT(@ColNameAsFilter2,1) = ''''
THEN SUBSTRING(REPLACE(@ColNameAsFilter2,'''''',''''),2,LEN(REPLACE(@ColNameAsFilter2,'''''',''''))-2)
ELSE @ColNameAsFilter2 END
--====================================================
-- Check that the first filter column name is valid
IF @ColNameAsFilter1 <> '' AND
NOT EXISTS (SELECT 1
FROM sys.all_columns
WHERE object_id = OBJECT_ID(@tableSchema+'.'+@tableName)
AND name = @ColNameAsFilter1)
BEGIN
RAISERROR (N'Table %s.%s does not have a column %s.',
16,
1,
@tableSchema,
@tableName,
@ColNameAsFilter1)
RETURN
END
--====================================================
-- Check that the second filter column name is valid
IF @ColNameAsFilter2 <> '' AND
NOT EXISTS (SELECT 1
FROM sys.all_columns
WHERE object_id = OBJECT_ID(@tableSchema+'.'+@tableName)
AND name = @ColNameAsFilter2)
BEGIN
RAISERROR (N'Table %s.%s does not have a column %s.',
16,
1,
@tableSchema,
@tableName,
@ColNameAsFilter2)
RETURN
END
--====================================================
-- Construct & execute the dynamic SQL
DECLARE @sqlCommand nvarchar(max)
SET @sqlCommand = 'SELECT ' + @columnList + CHAR(13) +
' FROM [' + @tableSchema + '].['+ @tableName + ']' + CHAR(13) +
' WHERE 1=1 '
IF @ColNameAsFilter1 != ''
SET @sqlCommand = @sqlCommand + CHAR(13) +
' AND ' + QUOTENAME(@ColNameAsFilter1) + ' = @ColFilter1VAL'
IF @ColNameAsFilter2 != ''
SET @sqlCommand = @sqlCommand + CHAR(13) +
' AND ' + QUOTENAME(@ColNameAsFilter2) + ' = @ColFilter2VAL'
EXECUTE sp_executesql @sqlCommand,
N'@ColFilter1VAL nvarchar(75), @ColFilter2VAL nvarchar(75)',
@ColFilter1VAL, @ColFilter2VAL
END
If you really want to go down this path, here is a starting point. It includes a free string splitting function. And uses NULLs rather than empty strings to indicate unused filter columns.
create function dbo.SplitCSL( @CSL as NVarChar(4000) )
-- Based on Jeff Moden's design.
returns table
with schemabinding as
return
with Digits as ( select Digit from ( values (0), (1), (2), (3), (4), (5), (6), (7), (8), (9) ) as Digits( Digit ) ),
Numbers as ( select ( ( Ten_3.Digit * 10 + Ten_2.Digit ) * 10 + Ten_1.Digit ) * 10 + Ten_0.Digit + 1 as Number
from Digits as Ten_0 cross join Digits as Ten_1 cross join Digits as Ten_2 cross join Digits as Ten_3 ),
cteTally(N) as ( select 0 union all select top ( DataLength( IsNull( @CSL, 1 ) ) )
Row_Number() over ( order by ( select NULL ) ) from Numbers ),
cteStart(N1) as ( select N + 1 from cteTally where Substring( @CSL, N, 1 ) = N',' OR N = 0 )
select Item = Substring( @CSL, N1, IsNull( NullIf( CharIndex( N',', @CSL, N1 ), 0 ) - N1, 8000 ) )
from cteStart;
go
create procedure [dbo].[SafeSqlSP_SelectGivenTableWithOptionalColFilter]
@ColumnList VarChar(75) = '*',
@TableName SysName,
@ColNameAsFilter1 VarChar(75) = NULL,
@ColNameAsFilter2 VarChar(75) = NULL,
@ColFilter1Val VarChar(75) = '',
@ColFilter2Val VarChar(75) = ''
as
begin
set nocount on;
declare @Schema as SysName = 'dbo';
-- Validate the table name.
if not exists ( select 42 from Information_Schema.Tables where Table_Schema = @Schema and Table_Name = @TableName )
begin
RaIsError( '@TableName does not reference a valid table or view.', 13, 0 );
return;
end;
-- Validate and QUOTENAME the column list.
declare @SelectList as NVarChar(1000);
if @ColumnList = '*'
set @SelectList = '*'
else
begin
declare @Columns as Table ( ColumnName SysName );
insert into @Columns
select Item from dbo.SplitCSL( @ColumnList );
if exists ( select 42 from @Columns as C left outer join
Information_Schema.Columns as ISC on ISC.Table_Schema = @Schema and ISC.Table_Name = @TableName and ISC.Column_Name = C.ColumnName
where ISC.Column_Name is NULL )
begin
RaIsError( '@ColumnList references an invalid column in the table or view.', 13, 0 );
return;
end;
-- Build a comma separated list ofquoted column names.
select @SelectList = Stuff(
( select N',' + QuoteName( ColumnName ) from @Columns for XML path(''), type).value('.[1]', 'NVarChar(1000)' ),
1, 1, '' );
end;
-- Validate the filter columns.
if @ColNameAsFilter1 is not NULL and
not exists ( select 42 from Information_Schema.Columns
where Table_Schema = @Schema and Table_Name = @TableName and Column_Name = @ColNameAsFilter1 )
begin
RaIsError( '@ColNameAsFilter1 does not reference a valid column in the table or view.', 13, 0 );
return;
end;
if @ColNameAsFilter2 is not NULL and
not exists ( select 42 from Information_Schema.Columns
where Table_Schema = @Schema and Table_Name = @TableName and Column_Name = @ColNameAsFilter2 )
begin
RaIsError( '@ColNameAsFilter2 does not reference a valid column in the table or view.', 13, 0 );
return;
end;
-- Build the parameterized query.
declare @SqlCommand as NVarChar(1000) =
'select ' + @SelectList + ' from ' + QuoteName( @Schema ) + '.' + QuoteName( @TableName ) + case
when @ColNameAsFilter1 is not NULL and @ColNameAsFilter2 is NULL
then ' where ' + QuoteName( @ColNameAsFilter1 ) + ' = @ColFilter1Val'
when @ColNameAsFilter1 is NULL and @ColNameAsFilter2 is not NULL
then ' where ' + QuoteName( @ColNameAsFilter2 ) + ' = @ColFilter2Val'
when @ColNameAsFilter1 is not NULL and @ColNameAsFilter2 is not NULL
then ' where ' + QuoteName( @ColNameAsFilter1 ) + ' = @ColFilter1Val and ' + QuoteName( @ColNameAsFilter2 ) + ' = @ColFilter2Val'
else ''
end
print 'SQL: ' + @SqlCommand; -- Debugging.
declare @ParameterDefinitions as NVarChar(1000) = '@ColFilter1Val VarChar(75), @ColFilter2Val VarChar(75)';
execute sp_ExecuteSql @SqlCommand, @ParameterDefinitions, @ColFilter1Val, @ColFilter2Val;
end
go
C# code to invoke the procedure would look something like this:
using (SqlConnection sqlConnection = new SqlConnection(yourConnectionString))
{
// Connect to the database.
sqlConnection.Open();
using (SqlCommand sqlCommand = new SqlCommand())
{
// Set up the stored procedure call.
sqlCommand.Connection = sqlConnection;
sqlCommand.CommandType = System.Data.CommandType.StoredProcedure;
sqlCommand.CommandText = "SafeSqlSP_SelectGivenTableWithOptionalColFilter";
sqlCommand.Parameters.AddWithValue("@ColumnList", "ShoeSize");
sqlCommand.Parameters.AddWithValue("@TableName", "Employees");
sqlCommand.Parameters.AddWithValue("@ColNameAsFilter1 ", "ManagerId");
sqlCommand.Parameters.AddWithValue("@ColFilter1Val ", "42");
// Execute the stored procedure.
SqlDataReader reader = sqlCommand.ExecuteReader();
// Process the results.
while (reader.Read())
{
Debug.Print((string)reader["ShoeSize"]);
}
reader.Close();
}
sqlConnection.Close();
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
