Parameterized Queries not working

Question

I had the following implementation of filling a DataTable with SQL:

var con = new SqlConnection();
var cmd = new SqlCommand();
var dt = new DataTable();
string sSQL = @"SELECT LogID, Severity, Title
                FROM dbo.Log
                WHERE UPPER(LogID) LIKE '%" + searchPhrase.ToUpper() + @"%'                        OR UPPER(Severity) LIKE '%" + searchPhrase.ToUpper() + @"%'                      OR UPPER(Title) LIKE '%" + searchPhrase.ToUpper() + @"%'                    ORDER BY " + orderBy + " " + orderFrom + @"
                OFFSET ((" + (Convert.ToInt32(current) - 1).ToString() + ") * " + rowCount + @") ROWS
                FETCH NEXT " + rowCount + " ROWS ONLY;";

try
{
    using (var connection = THF.Models.SQLConnectionManager.GetConnection())
    {
        using (var command = new SqlCommand(sSQL, connection))
        {
            connection.Open();
            command.CommandTimeout = 0;
            var da = new SqlDataAdapter(command);
            da.Fill(dt);
        }
    }
}
catch { }

This works nicely but I've realized that this is dangerous due to SQL Injection. So I've tried to solve that danger using parameterized queries like this:

var con = new SqlConnection();
var cmd = new SqlCommand();
var dt = new DataTable();

cmd.Parameters.Add(new ObjectParameter("@searchPhrase", searchPhrase.ToUpper()));
cmd.Parameters.Add(new ObjectParameter("@orderBy", orderBy));
cmd.Parameters.Add(new ObjectParameter("@orderFrom", orderFrom));
cmd.Parameters.Add(new ObjectParameter("@current", current));
cmd.Parameters.Add(new ObjectParameter("@rowCount", rowCount));

string sSQL = @"SELECT LogID, Severity, Title
                FROM dbo.Log
                WHERE UPPER(LogID) LIKE '%" + searchPhrase.ToUpper() + @"%'                        OR UPPER(Severity) LIKE '%" + searchPhrase.ToUpper() + @"%'                      OR UPPER(Title) LIKE '%" + searchPhrase.ToUpper() + @"%'                    ORDER BY " + orderBy + " " + orderFrom + @"
                OFFSET ((" + (Convert.ToInt32(current) - 1).ToString() + ") * " + rowCount + @") ROWS
                FETCH NEXT " + rowCount + " ROWS ONLY;";

try
{
    using (var connection = THF.Models.SQLConnectionManager.GetConnection())
    {
        using (var command = new SqlCommand(sSQL, connection))
        {
            connection.Open();
            command.CommandTimeout = 0;
            var da = new SqlDataAdapter(command);
            da.Fill(dt);
        }
    }
}
catch { }

Unfortunately now my data table doesn't fill. What am I doing wrong?

Answer 1

• You are using multiple command and connection references, not sure if thats a copy/paste problem or your actual code is like that. In the second case it will not even compile.
• Reference the parameters directly in your query, see below. Sql Server uses named parameters so the same parameter can be reused in multiple locations.
• Desc/Asc cannot be used as a parameter. You should double check the value though or use an enum and pass that (recommended).
• The same is true of the numeric values for rowcount, pass those in as numbers or check their values using a TryParse to ensure it is numeric and not malicious code.
• The default install options for Sql Server is for a case insensitive coalition. This means you do not have to UPPER a string to do a comparison. If you do have a case sensitive install then do not change this, otherwise remove all calls to UPPER when doing comparisons.
• Finally you well never know why your code is not working if you surround your code in try/catch and have an empty catch block. Your code will fail silently and you will be left scratching your head. Do not do this anywhere in your code, it is bad practice!! Either catch the exception and handle it (do something so code can recover) OR log it and rethrow using throw; OR do not catch it at all. I chose the later and removed it.

---

Code

var currentNum = Convert.ToInt32(current) - 1;
var temp = 0;
if(!"desc".Equals(orderFrom, StringComparison.OrdinalIgnoreCase) && !"asc".Equals(orderFrom, StringComparison.OrdinalIgnoreCase))
    throw new ArgumentException("orderFrom is not a valid value");
if(!int.TryParse(rowCount, out temp))
    throw new ArgumentException("Rowcount is not a valid number");

var dt = new DataTable();
string sSQL = @"SELECT LogID, Severity, Title
                FROM dbo.Log
                WHERE UPPER(LogID) LIKE @searchPhrase
                OR UPPER(Severity) LIKE @searchPhrase                      
                OR UPPER(Title) LIKE @searchPhrase                    
                ORDER BY @orderBy " + orderFrom + "
                OFFSET ((" + currentNum.ToString() + ") * " + rowCount + @") ROWS
                FETCH NEXT " + rowCount + " ROWS ONLY;";

using (var connection = THF.Models.SQLConnectionManager.GetConnection())
using (var command = new SqlCommand(sSQL, connection))
{
    cmd.Parameters.Add(new SqlParameter("@searchPhrase", "%" + searchPhrase.ToUpper() + "%"));
    cmd.Parameters.Add(new SqlParameter("@orderBy", orderBy));

    connection.Open();
    command.CommandTimeout = 0;
    var da = new SqlDataAdapter(command);
    da.Fill(dt);
}