In the following piece of code I am adding entries to an MS Access file using OleDB. The purpose of this post is to point out my bad programming practices and if I have created any security flaws here. I was reading somewhat about SQL injection attacks, and I wonder if this code might have any potential bugs.First, it's the AddEntry button in which I am sending a data depending on which type was chosen, and each type has its own SQL query.private void btnAddEntry_Click(object sender, EventArgs e){    // Multiple level field validations.    if (cmbType.SelectedIndex != -1)    {        if (cmbType.SelectedIndex == 0 &&            (!string.IsNullOrEmpty(txtUserName.Text.Trim()) &&            !string.IsNullOrEmpty(txtPassword.Text.Trim())))        {            string SQL =                INSERT INTO PersonalData([Type], [UserName], [Password])  +                VALUES(@Type, @UserName, @Password);            InsertData(SQL);        }        else if (cmbType.SelectedIndex == 1 &&            (!string.IsNullOrEmpty(txtURL.Text.Trim()) &&            !string.IsNullOrEmpty(txtUserName.Text.Trim()) &&            !string.IsNullOrEmpty(txtPassword.Text.Trim())))        {            // Creating SQL string. Using [] will prevent any erros            // that might occur if any other names will be reserved words.            string SQL =                INSERT INTO PersonalData([Type], [URL], [UserName], [Password])  +                VALUES(@Type, @URL, @UserName, @Password);            InsertData(SQL);        }        else if (cmbType.SelectedIndex == 2 &&            (!string.IsNullOrEmpty(txtSoftwareName.Text.Trim()) &&            !string.IsNullOrEmpty(txtSerialCode.Text.Trim())))        {            // Creating SQL string. Using [] will prevent any erros            // that might occur if any other names will be reserved words.            string SQL =                INSERT INTO PersonalData([Type], [SoftwareName], [SerialCode])  +                VALUES(@Type, @SoftwareName, @SerialCode);            InsertData(SQL);        }        else        {            lblMessage.Text = Please fill out all required fields!;        }    }    else    {        lblMessage.Text = Please select a type first!;    }}Secondly, this is actual code which inserts data into the file:private void InsertData(string sql){    // Initialising encrypting/decrypting file.    Security security = new Security();    using (OleDbConnection connection = new OleDbConnection())    {        // Creating command object.        connection.ConnectionString =            Provider=Microsoft.ACE.OLEDB.12.0; +            Data Source= + filePath + ; +            Persist Security Info=False; +            Jet OLEDB:Database Password= + hashPhrase.ShortHash(pass) + ;;        using (OleDbCommand command = new OleDbCommand(sql, connection))        {            // I need to make sure that I am using correct parameters            // when using specified combo type. If I wouldn't            // differenciate between types, then I would end up            // with empty fields in the database file.            if (cmbType.SelectedIndex == 0)            {                OleDbParameter prmType = new OleDbParameter                    (@Type, security.EncryptAES(cmbType.Text, pass, user));                OleDbParameter prmUserName = new OleDbParameter                    (@UserName, security.EncryptAES(txtUserName.Text, pass, user));                OleDbParameter prmPassword = new OleDbParameter                    (@Password, security.EncryptAES(txtPassword.Text, pass, user));                command.Parameters.Add(prmType);                command.Parameters.Add(prmUserName);                command.Parameters.Add(prmPassword);            }            else if (cmbType.SelectedIndex == 1)            {                OleDbParameter prmType = new OleDbParameter                    (@Type, security.EncryptAES(cmbType.Text, pass, user));                OleDbParameter prmURL = new OleDbParameter                    (@URL, security.EncryptAES(txtURL.Text, pass, user));                OleDbParameter prmUserName = new OleDbParameter                    (@UserName, security.EncryptAES(txtUserName.Text, pass, user));                OleDbParameter prmPassword = new OleDbParameter                    (@Password, security.EncryptAES(txtPassword.Text, pass, user));                command.Parameters.Add(prmType);                command.Parameters.Add(prmURL);                command.Parameters.Add(prmUserName);                command.Parameters.Add(prmPassword);            }            else if (cmbType.SelectedIndex == 2)            {                OleDbParameter prmType = new OleDbParameter                    (@Type, security.EncryptAES(cmbType.Text, pass, user));                OleDbParameter prmSoftwareName = new OleDbParameter                    (@SoftwareName, security.EncryptAES(txtSoftwareName.Text, pass, user));                OleDbParameter prmSerialCode = new OleDbParameter                    (@SerialCode, security.EncryptAES(txtSerialCode.Text, pass, user));                command.Parameters.Add(prmType);                command.Parameters.Add(prmSoftwareName);                command.Parameters.Add(prmSerialCode);            }            try            {                connection.Open();                command.ExecuteNonQuery();            }            catch (Exception ex)            {                MessageBox.Show(Error:  + ex.Message);            }        }    }    // Refreshing state of main window.    mainWindow.DisplayFileContent(filePath);    lblMessage.Text = Data was successfully added.;    // Clearing all fields.    ClearFields();}
Adding entries to an MS Access file using OleDB
c#;security;sql injection;ms access
Completely unrelated to encryption, I would like to provide feedback on your btnAddEntry_Click and InsertData functions.The biggest problem I see here is that your database intelligence is partially in this button's code, and the insertion function has several checks on the UI. What's a button? It's a control put in an interface that a user can activate to perform an action. What's an insert query? It's a command sent to a database to add information. What's the link between the two? There really isn't one.The button is not your program. The form is not your program. And your program is not your form. The logic you wrote is.First, let's simplify your selection choices. What would happen if you had to add a new insertion type? You'd need to edit two functions. Lots of work, very easy to make mistakes. Instead, you can bind those choices to a list:public enum EntryType{    [StringValue()]    None,    [StringValue(User)]    User,    [StringValue(User and URL)]    UserAndURL,    [StringValue(Software)]    Software}The StringValue attribute is taken from here:public class StringValue : System.Attribute{    public string Value { get; set; }    public StringValue(string value)    {        Value = value;    }}Plus this function to get the attribute:using System;using System.Reflection;public static class StringEnum{    public static string GetStringValue(Enum value)    {        string output = String.Empty;        Type type = value.GetType();        FieldInfo fi = type.GetField(value.ToString());        StringValue[] attrs =           fi.GetCustomAttributes(typeof(StringValue),                                   false) as StringValue[];        if (attrs.Length > 0)            output = attrs[0].Value;        return output;    }}And this extension method to bind a ComboBox to an enum with string values    public static void BindToEnum<TEnum>(this ComboBox combo)    {        Dictionary<string, TEnum> enumList =                     new Dictionary<string, TEnum>();        foreach (var enumValue in Enum.GetValues(typeof(TEnum)))            enumList.Add(StringEnum.GetStringValue((Enum)enumValue), (TEnum)enumValue);        // Bind the custom type combo box        combo.DisplayMember = Key;        combo.ValueMember = Value;        combo.DataSource = new BindingSource(enumList, null);    }Finally, in your form's constructor:public frmPersonalDataEntry(){    InitializeComponent();    cmbType.BindToEnum<EntryType>();}Now we have a solid Enum to work with and that displays well. The basic idea with the btnAddEntry_Click function can just be a big switch:switch ((EntryType)cmbType.SelectedValue){    case EntryType.None:    //do stuff        break;    case EntryType.User:    //do stuff        break;    case EntryType.UserAndURL:    //do stuff        break;    case EntryType.Software:    //do stuff        break;}Your database logic could be in a separate class, which I called PersonalDataEntry. I use a PersonalDataEntry object, with a constructor like:public PersonalDataEntry(string user, string pass, string filePath)This will handle your recording. Your form should have one object of this class, it doesn't need to be remade every entry.In InsertData, do not repeat your switch. InsertData should not be aware of a form. I put InsertData in the PersonalDataEntry class:private bool InsertData(string insertQuery, string[] paramNames, object[] paramValues, out string message){    message = ;    // Initialising encrypting/decrypting file.    Security security = new Security();    using (OleDbConnection connection = new OleDbConnection())    {        // Creating command object.        connection.ConnectionString = String.Format(connectionString, filePath, hashPhrase.ShortHash(pass));        using (OleDbCommand command = new OleDbCommand(insertQuery, connection))        {            for (int i = 0; i < paramNames.Length; i++)                command.Parameters.Add(new OleDbParameter(paramNames[i],security.EncryptAES(paramValues[i].ToString(),pass,user)));            try            {                connection.Open();                command.ExecuteNonQuery();                return true;            }            catch (Exception ex)            {                message = Error:  + ex.Message;                return false;            }        }    }}The trick is to just reassemble the rest. Each do stuff is to be replaced with a function in the PersonalDataEntry object that will handle the logic (query text, variables, checking if text is valid, etc). I have uploaded a full demo project.