HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Adding entries to an MS Access file using OleDB

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileoledbaddingusingaccessentries

Problem

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]) "

Solution

I can't see any SQL vulnerabilities, but I see a few crypto mistakes.

  • You're using the same password for the database login and the contents. You shouldn't reuse passwords, because it increases the risk of leakage.



  • AES has two parameters: the key and the initialisation vector (IV). Even when you use the same key, you should really be using a different IV to avoid correlation. (E.g. the same @Type value encrypted under the same key with the same IV will be the same; if you use a different IV, you can avoid an attacker being able to match them up). It would be better to get a random IV (using a secure RNG) and store that in another column.



  • You're using AES to conceal the password. With very few exceptions, you should hash passwords rather than encrypt them. The standard recommendation is to use bcrypt.

Context

StackExchange Code Review Q#6496, answer score: 5

Revisions (0)

No revisions yet.