Troy Bryant
Troy Bryant

Reputation: 1024

Securing data before update

I have a few text boxes that on my page that can add data to my database. What I'm looking for is how to actually make it more secure. The basic error checking of empty text boxes I know how to check. What I'm really searching for is bad data being saved or special characters like " ' " or " - ". Its a simple application only myself and maybe two other people will use it but still want to make sure learning and coding correctly. Any suggestions. This is my save button.

Here is my code:

 try
            {

                OleDbConnection conn = new OleDbConnection(@"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=H:\Databases\AddressBook.mdb");

                conn.Open();
                DataSet ds = new DataSet();

                string cmd = "SELECT * FROM tblAddressBook";

                OleDbDataAdapter da = new OleDbDataAdapter(cmd, conn);

                da.Fill(ds, "Display");

                DataRow newRow = ds.Tables["Display"].NewRow();

                newRow[1] = txtFirstName.Text;
                newRow[2] = txtLastName.Text;
                newRow[3] = txtEmail.Text;
                newRow[4] = txtPhone.Text;

                ds.Tables["Display"].Rows.Add(newRow);

                OleDbCommandBuilder cb = new OleDbCommandBuilder(da);
                cb.DataAdapter.Update(ds.Tables["Display"]);

                conn.Close();
                GridView1.DataBind();
            }
            catch (Exception)
            {
                lblErrorSave.Text = "Your information did not save clear form and try again";
            }

Upvotes: 0

Views: 55

Answers (1)

simon at rcl
simon at rcl

Reputation: 7344

Your code as shown is secure, but does have problems.

What your question is about is SQL Injection. This arises where you use dynamic SQL, like so (air code):

string sql = "insert into tableA (cola, colb) values ("
             + "'" + txtBox1.Text + "',"
             + "'" + txtBox2.Text + "')";

...and then go and execute it. Depending on the contents of the text boxes you could have all sorts of things happening. Something like "');drop table tableA;--" This does not happen when you use a DataSet as above, so that's OK

Hoewever, your code is very inefficient. The first thing you do is pull down the whole of the Address table. If this is any size it will be slow and add a lot of IO, memory, and computation to the procedure.

You are also not checking that the row to be entered is actually a new one, not a modification of an old one or a duplicate. This may or may not be important to your app, but usually is important (dup data can be a real pain). You can amend your read of the Address table to pull down e.g. a row with the same email address (or whatever is unique), and if it gets it then amend with new data as you do above.

However if the data is to be added, then you need to use parameters; Air Code again:

string sql = "insert into table (colA, colB) values (@colA, @colB)";
using (OleDbCommand com = new OleDbCommand(sql, conn))
{
    com.Parameters.Add("@colA", txtBox1.Text);
    com.Parameters.Add("@colB", txtBox2.Text);

    com.ExecuteNonQuery();

}

(Note that different drivers have slightly different syntax on adding Parameters and I'm not sure that the OleDb command supports this syntax, but there will be something close.)

Using Parameters prevents SQL Injection, as the values of the parameters are transported not intermixed in the SQL string and so their content has no effect of the SQL eventually executed.

Upvotes: 1

Related Questions