is there a more efficient way to write this sql code?

Question

protected void Button1_Click(object sender, EventArgs e)
{
    MySqlConnection connection = new MySqlConnection(ConfigurationManager.ConnectionStrings["myconstring"].ConnectionString);
    connection.Open();
    symptons = String.Join(", ", CheckBoxList1.Items.Cast<ListItem>().Where(i => i.Selected).Select(i => i.Value).ToArray());
    Label3.Text = symptons;
    if(symptons!="")
    {
        MySqlCommand cmd = new MySqlCommand("select d.dname from disease d inner join diseasesymptom ds on ds.did = d.did inner join symptom s on s.sid = ds.sid where s.sname in (" + symptons + ")", connection);
        using (MySqlDataAdapter sda = new MySqlDataAdapter())
        {
            cmd.Connection = connection;
            sda.SelectCommand = cmd;
            using (DataTable dt = new DataTable())
            {
                sda.Fill(dt);
                GridView1.DataSource = dt;
                GridView1.DataBind();
            }
        }
    }
    else
    {
        Label2.Text = "select at least one symptom";
    }      
}

I know I'm sort of performing sql injection on my own code so how can i prevent this, there are basically 3 tables:

disease_table [columns=] (disease_id,disease_name)
symptom_table [columns=] (symptom_id,symptom_name)
disease_symptom [columns=] (disease_id,symptom_id)

there is a checkboxlist on my web page which has symptoms where text=fever, value='fever'..and so on reason for doing this was that the users could select any number of checkboxes and in clause doesn't accept parameters

**warning** your code is extremely vulnerable to sql injection attacks! — Daniel A. White, Feb 11 '16 at 18:23
that's what I am saying how can I improve it and achieve the aforementioned result..people should read the question fully before down voting — panman, Feb 11 '16 at 18:37
maybe they down vote cause you didn't read the first page of google on ["how to avoid sql injection in c#"](https://www.google.com.mx/search?q=how+to+avoid+sql+injection+in+c%23&ie=utf-8&oe=utf-8&gws_rd=cr&ei=ita8Vq-yE4S1-QHcxKfwDQ) — Just Do It, Feb 11 '16 at 18:44
actually I went through it and learned about stored procedure but I am still a student so did not understand the concepts quite well..hence posted the question here — panman, Feb 11 '16 at 18:49
Then ask how to properly make use of stored procedures, although that's also on the first page of google. — Just Do It, Feb 11 '16 at 18:51
[call mysql stored procedure in c#](http://stackoverflow.com/questions/23507767/call-mysql-stored-procedure-in-c-sharp) — Just Do It, Feb 11 '16 at 18:55

score 0 · Answer 1 · edited Feb 13 '16 at 18:04

Contrary to the suggestions in the comments, you shouldn't need a stored procedure to protect against sql injection. You can use parameterized queries to dothat.

The following code should do the job:

MySqlCommand cmd = new MySqlCommand();
cmd.Connection = connection;

List<string> names = new List<string>();
for (int i = 0; i < symptons.Length; i++)
{
    names.Add("@Param_" + i);
    cmd.Parameters.Add(new MySqlParameter("@Param_" + i, symptons[i]));
}
cmd.CommandText = "select d.dname from disease d inner join diseasesymptom ds on ds.did = d.did inner join symptom s on s.sid = ds.sid where s.sname in (" + string.Join(",", names) + ")";

Basically you don't inject the values, but inject the parameter for the query instead. The parameter names are generated in your code so they can't be messed with. The values for the parameters are sanitized by the driver before the query is executed so those can't be messed with either.

is there a more efficient way to write this sql code?

1 Answers1