Converting multiple query to use parameters to avoid SQL injection

I have some dropdownlist in my aspx page and I am using the choices from them in my SQL query:

```
query = "";
DataTable taskData = new DataTable();
connString = @""; //connection string
strClause = "";

if (!blOnLoad)
{
if (ddlTaskName.SelectedIndex > 0) //dropdownlist
{
strClause += " AND CT.ATTR2739 LIKE '%" + ddlTaskName.SelectedItem.Text + "%'";
//strClause += string.format();
}
else
{
strClause += " AND (CT.ATTR2739 LIKE '%' OR CT.ATTR2739 IS NULL)";
}
if (ddlService.SelectedIndex > 0) //dropdownlist
{
strClause += " AND SE.ATTR2821 LIKE '%" + ddlService.SelectedItem.Text + "%'";
}
else
{
strClause += " AND (SE.ATTR2821 LIKE '%' OR SE.ATTR2821 IS NULL)";
}
if (ddlStatus.SelectedIndex > 0) //dropdownlist
{
strClause += " AND CT.ATTR2812 LIKE '%" + ddlStatus.SelectedItem.Text + "%'";
}
else
{
strClause += " AND (CT.ATTR2812 LIKE '%' OR CT.ATTR2812 IS NULL)";
}
if (ddlDueDate.SelectedIndex > 0) //dropdownlist
{
strClause += " AND CONVERT(VARCHAR(14), CT.ATTR2752, 110) LIKE '%" + ddlDueDate.SelectedItem.Text + "%'";
}
else
{
strClause += " AND (CONVERT(VARCHAR(14), CT.ATTR2752, 110) LIKE '%' OR CONVERT(VARCHAR(14), CT.ATTR2752, 110) IS NULL)";
}
if (ddlOwner.SelectedIndex > 0) //dropdownlist
{
strClause += " AND UA.REALNAME LIKE '%" + ddlOwner.SelectedItem.Text + "%'";
}
else
{
strClause += " AND (UA.REALNAME LIKE '%' OR UA.REALNAME IS NULL)";
}
if (ddlClient.SelectedIndex > 0) //dropdownlist
{
strClause += " AND C.ATTR2815 LIKE '%" + ddlClient.SelectedItem.Text + "%'";
}
else
{
strClause += " AND (C.ATTR2815 LIKE '%' OR C.ATTR2815 IS NULL)";
}
if (ddlSite.SelectedIndex > 0) //dropdownlist
{
strClause += " AND SI.ATTR2819 LIKE '%" + ddlSite.SelectedItem.Text + "%'";
}
else
{
strClause

The recommended way to avoid SQL injection attacks is to use parameters.
Also I can recommend that you create a stored procedure instead of using dynamic SQL.

You can pass your dropdown indexes as parameters to your SP and use the old trick of using it as conditional on the where clause.

Your select can end up like this:

select CT.columnA, CT.columnB
from tableA CT
join tableB SE on SE.idA = CT.id
where
(@ddlTaskName_SelectedIndex > 0 and (CT.ATTR2739 LIKE '%' + @ddlTaskName_SelectedItemText))
-- or (@ddlTaskName_SelectedIndex = 0 and (CT.ATTR2739 LIKE '%' OR CT.ATTR2739 IS NULL)) -- if you think enough ill see this line is unnecessary
and
(@ddlService_SelectedIndex > 0 and (SE.ATTR2821 LIKE '%' + @ddlService_SelectedItemText + '%')
-- This "else" line is unnecessary too, since we don't really filter it because eveything is "like %" or "null"


And so on ...

Those dropdown texts are still prone to SQL injection since a smart hacker may be able to change its value depending on your system.
You can fix it to just set variables inside the SP, this mean you will not pass the drop down text, just the indexes and do some swith case to set that varchar variables before the select.

That can add some maintenance issue, since your dropdown texts must match the same texts hardcoded on your SP. A better approach can be to create a domain table for each dropdown:

Create table DropDownClientValues
(
 index int
,text varchar(50)
)

And list your dropdown values directly from those domain tables; that way all your selects/SP can refer the exact same values.

This has the advantage of making adding/removing options from your drop downs very easy and with no impact in your code. Sadly this can be a bit onerous to refactor a big app that way.

StackExchange Code Review Q#61776, answer score: 10