parameterized queries vs. SQL injection

Question

I am new to Asp.net and I'm just starting to work with classes. I recently created a class that will handle most of my SQL queries for me so that I don't have to repeatedly create new connections over all my files.

One of the methods I've created takes in an SQL query as a parameter and returns the result. I know that I should be using parameterized queries to avoid SQL injections. My question is, how can I do this when I'm passing the query as a string parameter?

For example, here's a method I'll be calling:

public static DataTable SqlDataTable(string sql)
{
    using (SqlConnection conn = new SqlConnection(DatabaseConnectionString))
    {
        SqlCommand cmd = new SqlCommand(sql, conn);
        cmd.Connection.Open();
        DataTable TempTable = new DataTable();
        TempTable.Load(cmd.ExecuteReader());
        return TempTable;
    }
}

So from another file I'd like to use this method like so:

DataTable dt = new DataTable();

dt = SqlComm.SqlDataTable("SELECT * FROM Users WHERE UserName='" + login.Text  + "' and Password='" + password.Text + "'");

if (dt.Rows.Count > 0)
{
   // do something if the query returns rows
}

This works but would still be vulnerable to injections right? Is there a way I can pass the variables to the string as parameters? I know I can do this if I create a new SQLCommand object for the query and use Parameters.AddWithValue, but I wanted all my SQL commands to be in the separate class.

Answer 1

This works but would still be vulnerable to injections right?

Yeah, your code is terrifyingly vulnerable to SQL injections.

I know that I should be using parameterized queries to avoid SQL injections.

Oh absolutely yeah.

My question is, how can I do this when I'm passing the query as a string parameter?

You simply shouldn't be passing the query as a string parameter. Instead you should be passing the query as string parameter containing placeholders and the values for those placeholders:

public static DataTable SqlDataTable(string sql, IDictionary<string, object> values)
{
    using (SqlConnection conn = new SqlConnection(DatabaseConnectionString))
    using (SqlCommand cmd = conn.CreateCommand())
    {
        conn.Open();
        cmd.CommandText = sql;
        foreach (KeyValuePair<string, object> item in values)
        {
            cmd.Parameters.AddWithValue("@" + item.Key, item.Value);
        }

        DataTable table = new DataTable();
        using (var reader = cmd.ExecuteReader())
        {
            table.Load(reader);
            return table;
        }
    }
}

and then use your function like this:

DataTable dt = SqlComm.SqlDataTable(
    "SELECT * FROM Users WHERE UserName = @UserName AND Password = @Password",
    new Dictionary<string, object>
    {
        { "UserName", login.Text },
        { "Password", password.Text },
    }
);

if (dt.Rows.Count > 0)
{
   // do something if the query returns rows
}