parameterized queries vs. SQL injection

asp.net, c#

Solution

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
}

Problem

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.

Original source