Using OLEDB to connect to a MS Access 2007 file

Look at ANeves description of SQL injection.

I'll just show a solution to the problem:

First change the query to include a parameter placeholder, like this:

string query = "DELETE FROM PersonalData WHERE DataID = ?";

Later when you're building your query, you add the parameter to the OleDbCommand object:

var cmd = new OleDbCommand(query);
cmd.Parameters.Add(new OleDbParameter{Value = txtEntryID.Text};

... and then you're ready to execute the query as before.

The parameters mechanism of ADO.NET takes care of all known escapes that are needed to prevent a SQL-injection attack if you create you're queries right.

Your code can easily be exploited, as ANeves showed.

In your case a small fix can make it safe. By simply parsing the id as an integer, it's totally safe. An integer can not contain any harmful code:

int id = Int32.Parse(txtEntryID.Text);
string SQL = "DELETE FROM PersonalData WHERE DataID = " + id.ToString();

Concatenating the value into the SQL code is however generally considered bad practice. The general solution to handling user input in queries in a safe way is to use parameterised queries, like Arjan Einbu described.

You should still parse the input, though.

I made some change suggestions to your database method, making it reusable:

// Take a parameter that is a delegate for code that sets parameters
private static void ExecuteSql(string sql, Action<OleDbCommand> setParameters) {

  // Use 'using' to make sure that the objects are disposed correctly
  // No 'obj' prefix on variables, hungarian notation is not needed for type
  // No 'OleDb' in the variable name, that information is not needed in the name
  using (OleDbConnection connection = new OleDbConnection()) {

    objOleDbConnection.ConnectionString =
      "Provider=Microsoft.ACE.OLEDB.12.0;" +
      "Data Source=" + filePath + ";" +
      "Persist Security Info=False;" +
      "Jet OLEDB:Database Password=" + pass + ";";

    // Set query and connection in the command constructor
    using (OleDbCommand command = new OleDbCommand(sql, connection)) {

      // Call code that sets the parameters
      if (setParameters != null) {
        setParameters(objOleDbCommand);
      }

      try {
        connection.Open();
        command.ExecuteNonQuery();
      } catch (Exception ex) {
        MessageBox.Show("Error: " + ex.Message);
      }
      // No 'finally' block, as the 'using' block closes the connection

    }
  }
}

// Overload for no parameters
private static void ExecuteSql(string sql) {
  ExecuteSql(sql, null);
}

Usage:

// Parse the input
int id = Int32.Parse(txtEntryID.Text);
// Call the method with a lambda expression for setting parameters
// Set the parameter with type and value
ExecuteSql(
  "DELETE FROM PersonalData WHERE DataID = ?",
  command => {
    command.Parameters.Add("?", OleDbType.Integer).Value = id;
  }
);
// Handle UI stuff
mainWindow.DisplayFileContent(filePath);
lblMessage.Text = "Data was successfully deleted.";
txtEntryID.Clear();

Looking more at your code, I would also recommend using the using keyword instead of try/finally to close the connection.

using(var cn = new OleDbConnection(connectionString))
{
    // open your connection here if needed
    // run your querie(s) here
}  //cn.Dispose() is call here automatically, and that will also close the connection

This will improve the readability and understandability of your code by placing relevant code together, and also cut down on lines of code.

BTW since you're also using exception handling here, I would just put a try/catch around the using block.

I think readability of your code would increase if you used the constructor parameters more often.

Like:

var cn = new OleDbConnection(connectionString);

// instead of:
var cn = new OleDbConnection();
cn.ConnectionString = connectionString;

and

var cmd = new OleDbCommand(query);

// instead of:
var cmd = new OleDbCommand();
cn.CommandText = query;

Another solution would be to properly filter your input. It is easier, and it is also more dangerous, because it is easy to miss something.

For the example at hand, you need to parse the user input into a number, and then convert that number back to string in order to append it to your SQL string, as follows:

int dataid = int.Parse( txtEntryID.Text ); //will throw exception if not valid number
string SQL = "DELETE FROM PersonalData WHERE DataID = " + dataid; //implies dataid.ToString();

In the case of string parameters it gets more complicated, because you need to ensure that the string does not contain any characters below \u0020, that any single quotes within the string are properly escaped by performing s = s.Replace("'", "''"), and that any other characters that may have a special meaning under the RDBMS you are using are stripped away.