Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2005 Forums
 Transact-SQL (2005)
 Parameterized input not working

Author  Topic 

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-07 : 15:39:08
I am trying to code my program to ward off SQL injections by using parameterized user input. I cannot get the program to select any records using a LIKE command as shown below. Can anyone tell if there is something wrong with my code? It was working before I substituted the SearchName.Text field with the parameter @SearchNameparm.

string strSQL = "DECLARE @SearchNameparm varchar(20) ";
strSQL += "SELECT * FROM Obituary WHERE CemeteryCode = '" + Code.Text + "'";
if (searchname.CompareTo("All") != 0 && searchname.CompareTo("") != 0)
// old code strSQL += " AND LastName LIKE '" + SearchName.Text + "%'";
strSQL += " AND LastName LIKE @SearchNameparm";

OleDbDataAdapter adapter = new OleDbDataAdapter();
OleDbCommand selectCMD = new OleDbCommand(strSQL, OleDbConn1);
adapter.SelectCommand = selectCMD;
selectCMD.Parameters.Add("@SearchNameparm", OleDbType.VarChar, 20);
selectCMD.Parameters["@SearchNameparm"].Value = "'" + SearchName.Text + "%'";

dataSet1 = new DataSet();
int ret = adapter.Fill(dataSet1, "Obits");

BryanBurroughs
Starting Member

8 Posts

Posted - 2010-04-07 : 15:58:37
You don't need to include the single quotes when setting the value of a parameter. Fixing that will probably make it run
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-07 : 16:15:49
Thanks for your reply. I tried with and without quotes around the value and I still get no records returned. I have used parameterized input with update and insert commands successfully. This is the first time I have used them with a SELECT command and with a Fill.
Go to Top of Page

AndrewMurphy
Master Smack Fu Yak Hacker

2916 Posts

Posted - 2010-04-08 : 03:52:45
Why don't you use Stored Procedures? See previous advice here re the benefits.
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-04-08 : 04:05:58
If you are using dynamic SQL in your application to build a WHERE clause I recommend that you use SP_ExecuteSQL to execute it. This will cache the query plan, for performance, and you can call it with parameters just like an ordinary Stored Procedure (but have the benefit of building the WHERE clause dynamically).

Note that you should not use "SELECT *" but instead list the actual columns that you want to retrieve (i.e. only those used by your application) - otherwise someone may add monster TEXT column(s) in the future and every SELECT * query will be retrieving them, even if not used, which will be disastrous for performance - and probably take ages to fix at that time!

I think

selectCMD.Parameters["@SearchNameparm"].Value = SearchName.Text + "%";

should work - just make sure that the text entered by the user cannot exceed 19 characters (i.e. 20 in total) or make the declaration bigger ("DECLARE @SearchNameparm varchar(20)")
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-04-08 : 04:06:30
P.S. or change the SQL to be:

strSQL += " AND LastName LIKE @SearchNameparm + '%' ";
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-04-08 : 04:10:20
P.P.S. ACtually: Does your approach work? (I've never tried doing it that way).

You are passing a parameter from your application:

selectCMD.Parameters.Add("@SearchNameparm", OleDbType.VarChar, 20);
selectCMD.Parameters["@SearchNameparm"].Value = "'" + SearchName.Text + "%'";

but then also declaring it in your SQL:

string strSQL = "DECLARE @SearchNameparm varchar(20) ";

are they one-and-the-same? or do they conflict in some way?

As per my earlier suggestion about SP_ExecuteSQL - that would resolve that issue, but it may just be my ignorance of how to parametrise a simple SQL string
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 11:09:55
Thanks for the replies. I agree that I should not be retrieving all fields from the table. In fact, there are only 10 fields in the table and I am using 8 of them. I have successfully used parameter fields in UPDATE and INSERT command. This is the first time I tried them with a SELECT command and a fill. I will remove the redundancy of the parameter and try the example Kristen gave me. I have not used SP_ExecuteSQL but I will read up on it and try to use it in this example. Also, I have not used Stored Procedures because most of my programs require dynamic SQL since so much of the information is provided by the user for query purposes. There are 20 programs in this web application and all of them have to be reviewed for vulnerability to SQL Injection. I need to resolve this usage of parameters in order to complete the task. Thanks again for your help. I will post the results ASAP.
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-04-08 : 11:38:46
"I have not used Stored Procedures because most of my programs require dynamic SQL since so much of the information is provided by the user for query purposes"

Its a job made for SP_ExecuteSQL in that case, IMHO!

If you want to read a detailed account and all the ins-and-outs then have a look at http://www.sommarskog.se/dynamic_sql.html - excellent stuff on that site, but its not for the faint hearted!
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 13:26:55
I made an attempt to use sp_executesql and I keep getting an error message that says "Incorrect syntax near '000001' which is a useless message. My new code is shown below.

string strSQL = "DECLARE @userinput varchar(20);";
strSQL += "DECLARE @SQLString nvarchar(500);";
strSQL += "DECLARE @ParmDefinition nvarchar(500);";

string sqlstring = "N'SELECT * FROM Obituary WHERE CemeteryCode = '" + Code.Text + "'";
if (searchname.CompareTo("All") != 0 && searchname.CompareTo("") != 0)
{
sqlstring += " AND LastName LIKE @SearchNameparm";
searchvalue = SearchName.Text += "%";
}
else
searchvalue = "''";

sqlstring += "';";
strSQL += "SET @SQLString = " + sqlstring;
strSQL += "SET @ParmDefinition = N'@SearchNameparm nvarchar(20)';";
strSQL += "SET @userinput = " + searchvalue + ";";
strSQL += "EXECUTE sp_executesql @SQLString, @ParmDefinition, @SearchNameparm = @userinput;";

OleDbDataAdapter adapter = new OleDbDataAdapter();
OleDbCommand selectCMD = new OleDbCommand(strSQL, OleDbConn1);
adapter.SelectCommand = selectCMD;
dataSet1 = new DataSet();
int ret = adapter.Fill(dataSet1, "Obits");

I copied the sql string that was created from the above code and am displaying it below. If anyone can see where the syntax error is, please let me know.

DECLARE @userinput varchar(20);
DECLARE @SQLString nvarchar(500);
DECLARE @ParmDefinition nvarchar(500);
SET @SQLString = N'SELECT * FROM Obituary WHERE CemeteryCode = '000001'';
SET @ParmDefinition = N'@SearchNameparm nvarchar(20)';
SET @userinput = '';
EXECUTE sp_executesql @SQLString, @ParmDefinition, @SearchNameparm = @userinput;
Go to Top of Page

BryanBurroughs
Starting Member

8 Posts

Posted - 2010-04-08 : 13:39:41
quote:
SET @SQLString = N'SELECT * FROM Obituary WHERE CemeteryCode = '000001'';

looks like this line has 1 too many single quotes in it at the end, before the semicolon.
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 14:06:40
I need that quote there for the literal comparison to Cemetery Code. If I remove it I get another error stating there is an unclosed quotation mark.
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 14:44:20
I figured out a way to put in a dynamic parameterized variable into my sql code as shown below:

string sqlstring = " SELECT * FROM Obituary WHERE CemeteryCode = '" + Code.Text + "'";
string strSQL = "";

if (searchname.CompareTo("All") != 0 && searchname.CompareTo("") != 0)
{
strSQL = "DECLARE @SearchNameparm varchar(20); ";
strSQL += "SET @SearchNameparm = '" + SearchName.Text + "%';";
sqlstring += " AND LastName LIKE @SearchNameparm";
}
strSQL += sqlstring + " ORDER BY LastName ";
OleDbDataAdapter adapter = new OleDbDataAdapter();
OleDbCommand selectCMD = new OleDbCommand(strSQL, OleDbConn1);
adapter.SelectCommand = selectCMD;
dataSet1 = new DataSet();
int ret = adapter.Fill(dataSet1, "Obits");

Does this code prevent sql injection?
Go to Top of Page

Michael Valentine Jones
Yak DBA Kernel (pronounced Colonel)

7020 Posts

Posted - 2010-04-08 : 15:01:41
quote:
Originally posted by parrot

I am trying to code my program to ward off SQL injections by using parameterized user input. I cannot get the program to select any records using a LIKE command as shown below. Can anyone tell if there is something wrong with my code? It was working before I substituted the SearchName.Text field with the parameter @SearchNameparm.

string strSQL = "DECLARE @SearchNameparm varchar(20) ";
strSQL += "SELECT * FROM Obituary WHERE CemeteryCode = '" + Code.Text + "'";
if (searchname.CompareTo("All") != 0 && searchname.CompareTo("") != 0)
// old code strSQL += " AND LastName LIKE '" + SearchName.Text + "%'";
strSQL += " AND LastName LIKE @SearchNameparm";

OleDbDataAdapter adapter = new OleDbDataAdapter();
OleDbCommand selectCMD = new OleDbCommand(strSQL, OleDbConn1);
adapter.SelectCommand = selectCMD;
selectCMD.Parameters.Add("@SearchNameparm", OleDbType.VarChar, 20);
selectCMD.Parameters["@SearchNameparm"].Value = "'" + SearchName.Text + "%'";

dataSet1 = new DataSet();
int ret = adapter.Fill(dataSet1, "Obits");



What makes you think this will avoid SQL injection? It appears to be inserting user input directly into a SQL statement, the very definition of SQL injection.
strSQL += "SELECT * FROM Obituary WHERE CemeteryCode = '" + Code.Text + "'";

If you are expecting something like this:
SELECT * FROM Obituary WHERE CemeteryCode = 'value user typed in'

If the user types this in the text box: ';drop table Obituary; --
then you end up with:
SELECT * FROM Obituary WHERE CemeteryCode = '';drop table Obituary; --






CODO ERGO SUM
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 15:16:06
I thought if you paramterized the input, the system would process the parameter as a variable and not as executable code. If not, then how does one paramterize input into a SELECT statement? I've tried two other ways shown above, neither of which work. If you have a better way of doing it, let me know.
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 15:18:23
I should also mention that I am filtering out quotes, asteriks, double dashes, backslashes, etc. from the input before I even attempt to process the data.
Go to Top of Page

BryanBurroughs
Starting Member

8 Posts

Posted - 2010-04-08 : 15:32:35
Yes, if you correctly parameterize the code, it will avoid SQL injection. What you are doing, though, is NOT parameterizing.

SET @sql = N'some text ' + @param
in SQL is just as vulnerable as doing
String sql = 'some text ' + aTextBox.Text
in C# or VB.

if you are writing in VB or C#, you add the parameters using your OleDbCommand object, like you did in the first post. However, there is no need for the first line of SQL code where you declared @SearchNameparm. The OleDbCommand object does that for you automatically.
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 15:59:44
I inserted the DECLARE for @SearchNameparm because if I didn't I would get an error message saying 'Must declare the scalar variable "@SearchNameparm".' If I use the DECLARE statement and eliminate the selectCMD.Parameters.Add statement I also get an error saying that "@SearchNameparm' is not contained by this OleDbParameterCollection." So I can't win either way, I need both. It appears to me that the code in the first post is correct but I still get no records back.
Go to Top of Page

Michael Valentine Jones
Yak DBA Kernel (pronounced Colonel)

7020 Posts

Posted - 2010-04-08 : 16:02:17
You should read about it here.

Jeff's SQL Server Blog
Always Use Parameters. Even if you don't use Stored Procedures.
http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspx

CODO ERGO SUM
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-04-08 : 16:10:01
Parrot you are missing an important point with SP_ExecuteSQL:

You need to build two strings:

The Dynamic SQL Query statement - this will be something like:

string sqlQuery = " SELECT * FROM Obituary WHERE CemeteryCode = @CemeteryCode";

if (searchname.CompareTo("All") != 0 && searchname.CompareTo("") != 0)
{
sqlQuery += " AND LastName LIKE @SearchNameparm";
}
sqlQuery += " ORDER BY LastName ";

note that all "user provided data" is show as @Variables.

Next you need to prepare a string defining all possible parameters (it does not matter whether the Parameter is actually used in this particular query - such as @SearchNameparm in this example - you can just include all possible parameters (or you can do it programmatically to only include the actually-used parameters)

string sqlParameter = "@CemeteryCode varchar(99), @SearchNameparm varchar(20)";

Then you pass the Dynamic SQL query (sqlString), the Parameters list (sqlParameters) and then ALL the actual parameters, individually, to execute SP_ExecuteSQL (sorry, not familiar with your programming language, so I don't know the correct syntax, but it would eb the equivalent of:

EXEC sp_ExecuteSQL @sqlstring, @sqlParameter, @CemeteryCode, @SearchNameparm

You should never include any user-supplied data in the Dynamic SQL query (sqlString), only ever as a Parameter (defined in @sqlParameter and supplied as an additional parameter to the call to sp_ExecuteSQL.

Hope that is clear, but if not post your attempt and I expect we can help clarify.
Go to Top of Page

parrot
Posting Yak Master

132 Posts

Posted - 2010-04-08 : 16:59:26
Kristen;
I am programming in C# and have referenced examples of how to code sp_executesql. I understand that you should not include user input directly in an SQL statement. The cemetery code is really hard coded in a previous program and is carried through the various Sessions. I do think I am coding it with proper syntax. The code below shows the search for LastNme = Jones. I will keep trying to see if I can get rid of the syntax error that really doesn't tell me anything.

DECLARE @userinput varchar(20);
DECLARE @SQLString nvarchar(500);
DECLARE @ParmDefinition nvarchar(500);
SET @SQLString = N'SELECT * FROM Obituary WHERE CemeteryCode = '000001' AND LastName LIKE @SearchNameparm';
SET @ParmDefinition = N'@SearchNameparm nvarchar(20)';
SET @userinput = 'Jones';
EXECUTE sp_executesql @SQLString, @ParmDefinition, @SearchNameparm = @userinput;
Go to Top of Page
    Next Page

- Advertisement -