Problem with Dynamic SQL

I am using Dynamic SQL in stored procedures. This following procedure accepts a parameter @.filter which provides the WHERE clause for the query if there is one. Here is the code:

CREATE PROCEDURE TC3_GetAllJobOrders
(
@.filter NVARCHAR(500)
)
 AS
DECLARE @.sqlString NVARCHAR(500)
IF @.filter IS NOT NULL
BEGIN
SET @.sqlString = N'SELECT * FROM TC3_JobOrder WHERE @.filter'
EXECUTE sp_executesql @.sqlString, N'@.filter NVARCHAR(500)', @.filter
END
ELSE
BEGIN
SET @.sqlString = N'SELECT * FROM TC3_JobOrder'
EXECUTE sp_executesql @.sqlString
END
GO

When I pass in null for @.filter, it works fine and returns all rows in the TC3_JobOrder table. However when I pass in a syntactically correct filter I get the following error:

"Line 1: Incorrect syntax near '@.filter'."

Even a trivial filter like "1 = 1" gives me this error. I have changed the query from a SELECT statement to a RAISERROR statement so that it prints out the SQL after the value for @.filter has been injected, and everything is syntactically correct. Any ideas why it would be giving me that error? It all seems right...

Jason PachecoYou can't pass in the WHERE condition in that fashion. You just need to concatenate the variable to your SQL string:

IF @.filter IS NOT NULL
BEGIN
SET @.sqlString = N'SELECT * FROM TC3_JobOrder WHERE ' + @.filter
EXECUTE sp_executesql @.sqlString
END
ELSE
BEGIN
SET @.sqlString = N'SELECT * FROM TC3_JobOrder'
EXECUTE sp_executesql @.sqlString
END

Terri|||And I should add that this approach is extremely vulnerable to SQL injection. The @.Filter value should NOT be coming directly from user input.

Terri|||"And I should add that this approach is extremely vulnerable to SQL injection."

This is what I was afraid of... I posted previously about avoiding SQL injection and I did a little reading, but didn't find anything good. What can I do to avoid SQL injection?

Basically what I want to do is have filterable datagrids and I am using stored procedures for all of my SQL. So the only way I can see to have filterable datagrids (short of writing a stored procedure for every possible combination of filters) is to write a dynamic SQL statement that passes the WHERE clause in.

Is there a better solution? For instance, one thing I was thinking of is, instead of passing in the filter as one argument, I could pass in each field's filter value seperately. Howere, that would mean the stored procedure would take a ton of arguments, not to mention the fact that everytime the table definition changes, the stored procedure would have to change as well, along with the interface.

Jason Pacheco|||You could possibly do something like this:

SELECT
 *
FROM
 TC3_JobOrder
WHERE
 JobNumber = ISNULL(@.JobNumber,JobNumber) AND
 CustomerNumber = ISNULL(@.CustomerNumber,CustomerNumber) AND
 JobDate BETWEEN ISNULL(@.StartDate,'19000101') AND ISNULL(@.EndDate,GETDATE())

To prevent SQL injection you would need to completely validate the user's input, plus make sure that the SQL user account has limited permissions so that if an injection attack is successful the damange is minimized. This isn't the approach I normally take so I don't have firsthand experience.

But I can share with you something that David Penton advised last year on using Dynamic Where clauses, and I don't think he'd mind me sharing it here:
**************************************************************
Date: Tue, 10 Jun 2003 09:13:39 -0600
From: "David L. Penton"
To: aspnet-databases@.aspadvice.com
Subject: Re: [aspnet-databases] RE: Optional Stored Proc Query Parameters All headers

I have a method which I have been using which I like alot. Let's say you have 3 things you want to filter on, but you need to allow any combination of the three.
For simplicity's sake, let's also say that you require at least one value. I am also leaving a few things out for clarity:

CREATE PROCEDURE dbo.myProc
@.val1 int = NULL
, @.val2 varchar(10) = NULL
, @.val3 decimal(12, 0) = NULL
AS

SET NOCOUNT ON

DECLARE @.sql nvarchar(4000)

-- I require a WHERE clause, but you may want to
-- modify it to not need one
IF @.val1 IS NULL OR @.val2 IS NULL OR @.val3 IS NULL BEGIN
RAISERROR('Must have at least one value for @.val1, @.val2, or @.val3', 11,
1)
RETURN (1)
END

-- make base SQL statement
SET @.sql = N'
SELECT
a.val1, a.val2, a.val3, a.val4, a.val5 FROM
dbo.myTable a
WHERE
1 = 1
@.val1REP
@.val2REP
@.val3REP'

-- make logical replacements
IF @.val1 IS NOT NULL
SET @.sql = REPLACE(@.sql, '@.val1REP', 'AND a.val1 = @.val1') ELSE
SET @.sql = REPLACE(@.sql, '@.val1REP', '')

IF @.val2 IS NOT NULL
SET @.sql = REPLACE(@.sql, '@.val2REP', 'AND a.val2 = @.val2') ELSE
SET @.sql = REPLACE(@.sql, '@.val2REP', '')

IF @.val3 IS NOT NULL
SET @.sql = REPLACE(@.sql, '@.val3REP', 'AND a.val3 = @.val3') ELSE
SET @.sql = REPLACE(@.sql, '@.val3REP', '')

--select @.sql
EXECUTE "dbo"."sp_executesql"
@.sql
, N'@.val1 int, @.val2 varchar(10), @.val3 decimal(12, 0)'
, @.val1 = @.val1
, @.val2 = @.val2
, @.val3 = @.val3

RETURN 0

GO

This has a benefit of:

[] preventing SQL injection because you are using parameterized queries

[] caching query plans because you are using sp_executesql

[] controlling the data coming in to the procedure

I actually would do a little more with the '1=1' part, but for email clarity I am not.
Also, in my queries I typically have values that are always required so the '1=1' isn't necessary.

hth...

David L. Penton, Microsoft MVP SQL Server
JCPenney Technical Specialist / Lead
"Mathematics is music for the mind, and Music is Mathematics for the Soul. - J.S. Bach"
david@.davidpenton.com

Do you have the VBScript Docs or SQL BOL installed? If not, why not?
VBScript Docs: http://www.davidpenton.com/vbscript
New SP3 SQL BOL: http://www.davidpenton.com/sqlbol

**************************************************************

Also, I think this Patterns & Practices is a good read:Improving Web Application Security: Threats and Countermeasures,Building Secure ASP.NET Applications, in particularChapter 14 - Building Secure Data Access.

Terri|||Alright, I went with a parameterized stored procedure similar to what you have proposed. However, I have one remaining problem. One of the values I am passing in is tied directly to a textbox on the frontent and is very subject to SQL injection. What is the best way of validating this control before its value is passed to the sproc?

I thought about modifying permissions, but it's an ASP.NET application so the sproc runs in the context of the ASP.NET user, who already has EXEC permissions to the sproc, and that can't be modified.|||It took me a while to write that reply and it's not as organized as it could be otherwise.
I mixed my message somewhat. If you use parameters SQL injection should not be an issue.

Since I haven't used dynamic SQL which is built directly from user input I have not developed a strategy to handle this myself. I know you should validate input to make sure the data is of the datatype you are expecting. But if you have a freeform textbox where the user can type in " AND City = 'Dallas'" then that is really a gaping hole.

Hopefully others can chime in on this.

Here's a brief synopsis I found from SQL Server MagazineInjection Protection. And I see thatChapter 10 of the link I gave you previously has a lot of meaty information about validating input.

Terri