INSERTing a student info record based on form inputs

Question

I want to improve my coding skills. How do I write this code with concatenation

string query = "insert into StudentInfo values ('" + textBox1.Text.Trim() + "', '" + Name.Trim() + "', '" + comboBox1.SelectedItem.ToString() + "') ";
SqlCommand command = new SqlCommand(query, cn);
command.ExecuteNonQuery();

• 200_success: You just did write it with concatenation.
• Heslacher: You should never ever do something like this ! Sql injection

Answer 1

Like I had mentioned in my comment this is vulnerable to sql injection. Assume the user of your application inserts Robert'; DROP TABLE StudentInfo;-- that would result in a query text of "insert into StudentInfo values ('Robert','',''); DROP TABLE StudentInfo;-- ', 'theName', 'theComboboxValue') " resulting in deleting your database table StudentInfo which isn't what you want !

The solution to this is using SqlParameter's which will escape the provided values in a way that this can't happen like so

string query = "insert into StudentInfo values (@first, @second, @third)";
SqlCommand command = new SqlCommand(query, cn);
command.Parameters.AddWithValue("@first", textBox1.Text.Trim());
command.Parameters.AddWithValue("@second", Name.Trim());
command.Parameters.AddWithValue("@third", comboBox1.SelectedItem.ToString());
command.ExecuteNonQuery();

I have named the parameters first, second and third, because I don't know what their meaning is, so you should give them proper names yourself. The @ needs to be at the beginning so that the names can be parsed to the desired parameters.

Naming is very important in programming, so you should always use proper names for anything you do. If you come back in 2 weeks to this piece of code you won't know for what e.g textBox1 or comboBox1 are standing.

• RobH: FWIW it would need a value of Robert', '', ''); DROP TABLE StudentInfo;-- in textBox1 so the number of columns match.
• Heslacher: @RobH I assumed that the remaining columns can be null ;-)
• RobH: If you omit the column list (which you shouldn't) you have to specify all columns in the insert.
• Heslacher: Ok, fixed it. My sql is a little bit rusty.