0

I know there are other answers to SQL Injection in C#, but i'm new to programming so i have a hard time understanding how to set it up with my own code. That's why i have opened another question.

This is the code i have now, but i want to make it safe from SQL Injections.

    private void btnAdd_Click(object sender, EventArgs e)
    {
        string constring = $"Data Source=(LocalDB)\\MSSQLLocalDB;AttachDbFilename=" + Directory.GetCurrentDirectory().ToString() + "\\BarcodeDB.mdf;Integrated Security=True";
        string Query = "INSERT INTO Products (Barcodes, Name, EDate, Quantity, Price) VALUES ('" + this.tbxBar.Text + "','" + this.tbxName.Text + "','" + this.dateDate.Value.Date + "','" + this.tbxQua.Text + "','" + this.tbxPrice.Text + "') ;";
        SqlConnection conDataBase = new SqlConnection(constring);
        SqlCommand cmdDataBase = new SqlCommand(Query, conDataBase);
        SqlDataReader myReader;
        try
        {
            conDataBase.Open();
            myReader = cmdDataBase.ExecuteReader();
            while (myReader.Read())
            {

            }

            Fillcombo();

        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
        conDataBase.Close();
    }

If someone could explain or write the code so i can try to understand it by myself. I just don't seem to know what i'm supposed to change.

Thanks beforehand!

EDIT:

Changed the code to this now but it doesn't save it into the database

   private void btnAdd_Click(object sender, EventArgs e)
    {
        string constring = $"Data Source=(LocalDB)\\MSSQLLocalDB;AttachDbFilename=" +
                           Directory.GetCurrentDirectory().ToString() + "\\BarcodeDB.mdf;Integrated Security=True";
        string query =
           "INSERT INTO Products (Barcodes, Name, EDate, Quantity, Price) VALUES (@barcodeValue, @nameValue, @dateValue, @quantityValue, @priceValue) ;";
        SqlConnection conDataBase = new SqlConnection(constring);
        SqlCommand cmdDataBase = new SqlCommand(query, conDataBase);

        // Add the parameters to the command, setting the values as needed. Guessing at value types here.
        // Note that with strings you will need to define the length of the column in the DB in the assignment
        cmdDataBase.Parameters.Add("barcodeValue", SqlDbType.NVarChar, 255).Value = this.tbxBar.Text;
        cmdDataBase.Parameters.Add("nameValue", SqlDbType.NVarChar, 255).Value = this.tbxName.Text;
        cmdDataBase.Parameters.Add("dateValue", SqlDbType.Date).Value = this.dateDate.Text;
        cmdDataBase.Parameters.Add("quantityValue", SqlDbType.Int).Value = this.tbxQua.Text;
        cmdDataBase.Parameters.Add("priceValue", SqlDbType.Decimal).Value = this.tbxPrice.Text;

        SqlDataReader myReader;
        try
        {
            conDataBase.Open();
            myReader = cmdDataBase.ExecuteReader();
            while (myReader.Read())
            {
            }

            Fillcombo();
        }
LooChar
  • 1
  • 2
  • https://msdn.microsoft.com/en-us/library/ff648339.aspx – abr Oct 16 '17 at 11:28
  • 5
    Possible duplicate of [How can I add user-supplied input to an SQL statement?](https://stackoverflow.com/questions/35163361/how-can-i-add-user-supplied-input-to-an-sql-statement) – Dirk Oct 16 '17 at 11:30
  • @Dirk it is a duplicate, if you read the first paragraph of his post, you can understand why he posted this regardless. – FrankK Oct 16 '17 at 11:31
  • Don't use string concatenation to build your SQL statements but instead stick to parameterized queries or OR mappers. – Peit Oct 16 '17 at 11:32
  • @Peit That's the problem i'm at now i don't really understand how i do that – LooChar Oct 16 '17 at 11:35
  • If you have trouble understanding SQL injection, take into account that the hacker is closing *your* quotation marks and adding a SQL command like DELETE, then he opens quotation marks again for preventing syntax error. This is basically the trick. The command ends up outside quotation and runs. – derloopkat Oct 16 '17 at 11:36
  • @derloopkat yeah i figured it was something like that but my problem is that i don't know how to fix it so it won't be vunerable to it – LooChar Oct 16 '17 at 11:38
  • see above link, use parameters. – derloopkat Oct 16 '17 at 11:39
  • 1
    As you are performing an insert command, with no Select command after it - you should be using ExecuteNonScalar. As far as I can see though, the command should be excuted - you just will not have anything to read from the result. – PaulF Oct 16 '17 at 12:34

2 Answers2

1
    string Query = "INSERT INTO Products (Barcodes, Name, EDate, Quantity, Price) VALUES ('" + this.tbxBar.Text + "','" + this.tbxName.Text + "','" + this.dateDate.Value.Date + "','" + this.tbxQua.Text + "','" + this.tbxPrice.Text + "') ;";

This line is the danger line; assuming your textboxes are accepting user input, I could easily type in some code which would execute against your DB, potentially wiping it, dumping the contents out to the screen, or any number of other mischief or damage scenarios.

The way you protect against it is to use parameters. They will ensure that what you pass in is the correct value, and throw an exception before you ever get to the database. Much better to have an exception thrown before the malicious user gets a chance to execute bad code against your DB.

Something like the below should work, though you may need to change types to suit your DB scenario. Note how your query uses the "@variableName" pattern now, and how those variable names are used later to add parameters to the SqlCommand.

private void btnAdd_Click(object sender, EventArgs e)
{
   string constring = $"Data Source=(LocalDB)\\MSSQLLocalDB;AttachDbFilename=" +
                      Directory.GetCurrentDirectory().ToString() + "\\BarcodeDB.mdf;Integrated Security=True";
   string query =
      "INSERT INTO Products (Barcodes, Name, EDate, Quantity, Price) VALUES (@barcodeValue, @nameValue, @dateValue, @quantityValue, @priceValue) ;";
   SqlConnection conDataBase = new SqlConnection(constring);
   SqlCommand cmdDataBase = new SqlCommand(query, conDataBase);

   // Add the parameters to the command, setting the values as needed. Guessing at value types here.
   // Note that with strings you will need to define the length of the column in the DB in the assignment
   cmdDataBase.Parameters.Add("barcodeValue", SqlDbType.NVarChar, 255).Value = this.tbxBar.Text;
   cmdDataBase.Parameters.Add("nameValue", SqlDbType.NVarChar, 255).Value = this.tbxName.Text;
   cmdDataBase.Parameters.Add("dateValue", SqlDbType.Date).Value = this.dateDate.Text;
   cmdDataBase.Parameters.Add("quantityValue", SqlDbType.Int).Value = this.tbxQua.Text;
   cmdDataBase.Parameters.Add("priceValue", SqlDbType.Decimal).Value = this.tbxPrice.Text;

   SqlDataReader myReader;
   try
   {
      conDataBase.Open();
      myReader = cmdDataBase.ExecuteReader();
      while (myReader.Read())
      {
      }

      Fillcombo();
   }

   catch (Exception ex)
   {
      MessageBox.Show(ex.Message);
   }

   conDataBase.Close();
}
Marisa
  • 732
  • 6
  • 22
0

Use this code when creating your Query string:

        string Query = "INSERT INTO Products (Barcodes, Name, EDate, Quantity, Price) VALUES (@bar, @name, @date, @qua, @price) ;";

and this to create and set up your SqlCommand to use that query:

        SqlCommand cmdDataBase = new SqlCommand(Query, conDataBase);

        cmd.Parameters.AddWithValue("@bar", this.tbxBar.Text);
        cmd.Parameters.AddWithValue("@name", this.tbxName.Text);
        cmd.Parameters.AddWithValue("@date", this.dateDate.Value.Date);
        cmd.Parameters.AddWithValue("@qua", this.tbxQua.Text);
        cmd.Parameters.AddWithValue("@price", this.tbxPrice.Text);

Then you can use the rest of your code the same way.

kenny_k
  • 3,831
  • 5
  • 30
  • 41