sp_executeSQL - Avoid Injection risk?

[lameid]

My understanding is that the SQL injection risk from contantenating strings together comes from concatenating text into the command string rather than using parameters.

To pass multiple optional paramaters for criteria to be added to a select (anded together), couldn't you use sp_executeSQL to pass all the parameters and only optionally include parameters in the statement to be executed? That is concatenate in the text to add a parameter rather than the value of the parameter. For example, below. I want to do this for 8 or so parameters... Am I missing something fundamental or is this safe?

CREATE PROCEDURE dbo.usp_SchedVisit (
	
	@Col1_IN nvarchar(255) = '', @Col2_IN int = -1
) 

AS
Declare @SQLString Nvarchar(900)
Declare @ParmDefinition Nvarchar(250)
Declare @Criteria Nvarchar(500)

Set @SQLString = 'SELECT 
	Table.Col1
	Table.Col2
FROM dbo.Table'

/*Just because you pass a bunch of parameters to sp_ExecuteSQL, 
that does not mean you have to use them all (for criteria)*/

/*Conditionally add Parameters for values passed in*/
Set @Criteria = ''

IF @Col1_IN <> ''
	Set @Criteria = ' Where Col1 = @Col1'

IF @Col2_IN <> -1
	Begin
		IF LEN(@Criteria) = 0
			Set @Criteria = ' Where Col2 = @Col2'
		Else
			Set @Criteria = @Criteria + ' And ' + 'Col2 = @Col2'
	End 

Set @SQLString = @SQLString + ' ' +  @Criteria
Set @ParmDefinition = N'@Col1 nvarchar(255), @Col2 int'


exec sp_executeSQL @SQLString, 
	@ParmDefinition, 
	@Col1 = @Col1_IN,
	@Col2 = @Col2_In
	;

 

[lameid]

I ran across an article that supports the above methodology... Also includes a static SQL option.

http://www.sommarskog.se/dyn-search-2008.html

 

[markros]

The above is safe and a good practice.

PluralSight Learning Library

 

[Olaf Doschke]

agreed.

This kind of dynamically putting together sql is offering a good overview over the possible results and as such is as good as one static sql with parameters. You're just avoiding to write three sqls and as you say your real case is about 8 optional parameters and optional criteria of the where clause, this will keep you from writing 128 different sqls, or one, where you need to care for some default value leading to no filtering.

It is a good practice, indeed.

You can also go another route: eg querying for a datetime range can either filter that with two given start and end datetimes or you set them to default values with two dates way before the earliest and past the last date.

It's just a matter of performance and indeed precompiled statements can run faster even with such unneeded criteria. As long as you can filter or not filter with the same static sql this can be better performacewise.

Bye, Olaf.

 

[davejam]

You could just pass null values for the criteria that you don't want

ie.

select field 
from table
where field1 = isnull(@Var1,field1)

so if it does not have criteria passed it just says where it is the same as itself!!


daveJam

easy come, easy go!!!

 

[gmmastros]

davejam,

This is only true if the column does not allow NULLS. If it allows NULL, you will not get all of the rows.

Take a look at this code:

Declare @Temp Table(ID Int, Data VarChar(20))

Insert Into @Temp Values(1, 'Apple')
Insert Into @Temp Values(2, '')
Insert Into @Temp Values(3, NULL)

Declare @Var1 VarChar(20)

Set @Var1 = NULL

Select *
From   @Temp
Where  Data = IsNull(@Var1, Data)

When you run this code, you will only get rows 1 and 2. You will not get row 3 because DATA is NULL.


-George
Microsoft SQL Server MVP
My Blogs
SQLCop
twitter
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom