Best way to replace EXECUTE(@Query)

Question

Post reply

Best way to replace EXECUTE(@Query)

Wobble Chops

SSC Eights!

Points: 835
More actions
November 9, 2010 at 9:04 am

#226233

Hi guys,
I have a string of comma separated integers coming from a web form which I need to use as a 'WHERE IN' clause:
The following works fine but I have been tasked with replacing the EXECUTE command since it leaves us open to direct injection attacks.
DECLARE @IDList nvarchar(1000)
DECLARE @query nvarchar(2000)
SELECT @IDList = N'100,101,102'
EXECUTE(N'SELECT * FROM #Temp1 WHERE pkID IN (' + @IDList + ')')
I was hoping to use something like the code below but I get the error:
Conversion failed when converting the nvarchar value '100,101,102' to data type int
EXECUTE sp_executesql N'SELECT * FROM TW_Transaction WHERE pkTransactionID IN (@param1)',
N'@param1 nvarchar(1000)',
@param1 = @IDList;
Test table for example
CREATE TABLE #Temp1(
pkID int
)
INSERT INTO #Temp1
SELECT 100
UNION
SELECT 101
UNION
SELECT 102
UNION
SELECT 103
Any help would be greatly appreciated.
Paul.

Viewing 6 posts - 1 through 5 (of 5 total)

You must be logged in to reply to this topic. Login to reply

Michael Valentine Jones SSC Guru Points: 64818 More actions · Answer 1

There is no need for dynamic SQL.

Insert the values in @IDList into a table, and then just do this:

SELECT * FROM TW_Transaction WHERE pkTransactionID IN (select ID from #temp)

You can find pleny of examples on this site of code to insert the values from a delimited list into table.

Ron McCullough SSC Guru Points: 63877 More actions · Answer 2

You can find pleny of examples on this site of code to insert the values from a delimited list into table.

Might I suggest this article by Jeff Moden, complete with sample code:

http://www.sqlservercentral.com/articles/T-SQL/63003/

If everything seems to be going well, you have obviously overlooked something.

Ron

Please help us, help you -before posting a question please read[/url]
Before posting a performance problem please read[/url]

WayneS SSC Guru Points: 95450 More actions · Answer 3

SELECT Item = convert(int, ds.Item)

FROM dbo.DelimitedSplit8k(@IDList, ',') ds;

Click here for the latest Delimited Split Function.

Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes

If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!

Links:
For better assistance in answering your questions
Performance Problems
Common date/time routines
Understanding and Using APPLY Part 1 & Part 2

Wobble Chops SSC Eights! Points: 835 More actions · Answer 4

Many thanks guys.

I wondered if there was a better way than getting involved with temp tables and things but I guess not. Tally table it is then 🙂

Thanks again for your input. It's much appreciated.

Paul.

lenne_dk SSC Journeyman Points: 92 More actions · Answer 5

Wobble Chops (11/9/2010)
Hi guys,
I have a string of comma separated integers coming from a web form which I need to use as a 'WHERE IN' clause:
The following works fine but I have been tasked with replacing the EXECUTE command since it leaves us open to direct injection attacks.
SELECT @IDList = N'100,101,102'
EXECUTE(N'SELECT * FROM #Temp1 WHERE pkID IN (' + @IDList + ')')

While "concatenated queries" are generally a bad thing, they can be used safely, if the parameters are cleaned properly.

in vbscript:

aList=split(IDList,",")

for n=lBound(aList) to uBound(aList)

aList(n)=cLng(aList(n))

next

IDListClean=join(aList,",")

This make sure IDListClean only contains numbers separated with commas, so no injections are possible.

To make the snippet more robust, a IsNumeric() test could be added, and other sanity checks like catching negative numbers, however that would probably not make any harm.

Or limiting the number of allowed numbers.