1

I have a registration page where the user can enter email into mail.Text and password into pass.Text, but when I click the enter button I get an error, it seems the sytem thinks the hashed password and the email address are column names so I get inavlid column names followed by the entered email and hashed password, I'm not sure what I've done wrong but I think it may be to do with my INSERT function. 

private void enterdetails_Click(object sender, EventArgs e)
{
    string query = String.Format("INSERT INTO tbl_register(email,password) VALUES({0},{1})",
        mail.Text,Hashing.ComputeHash(pass.Text, Supported_HA.SHA256,  
        ASCIIEncoding.ASCII.GetBytes("Supported_HA.SHA256")));
    SqlConnection sqlconn = new SqlConnection(@"");
    sqlconn.Open();
    SqlCommand comm = new SqlCommand(query, sqlconn);
    comm.ExecuteNonQuery();
    sqlconn.Close();
}
Steve
  • 213,761
  • 22
  • 232
  • 286
craibson
  • 11
  • 1
  • 5
    Use parameters to avoid sql injection and formatting errors. – LarsTech Mar 06 '19 at 19:05
  • SHA* is too fast for passwords; use bcrypt. – SLaks Mar 06 '19 at 19:05
  • 1
    The reason of your error is simple and you could have avoided it if you had used the parameters – Steve Mar 06 '19 at 19:06
  • password as a column name is also a bad idea – Ňɏssa Pøngjǣrdenlarp Mar 06 '19 at 19:08
  • Always use parameterized sql and avoid string concatenation to add values to sql statements. See [How can I add user-supplied input to an SQL statement?](https://stackoverflow.com/q/35163361/1260204), and [Exploits of a Mom](https://xkcd.com/327/). – Igor Mar 06 '19 at 19:10
  • Also wrap your connection instances in `using` blocks (that goes for any instance where `IDisposable` is implemented by the type). – Igor Mar 06 '19 at 19:11
  • Also `password` and `email` are reserved words in some RDBMS's. You should escape them using brackets. `([email], [password])` – Igor Mar 06 '19 at 19:13
  • Thanks for the information I will look into what you guys have mentioned, this is just for a homework assignment but I did realise with research that there are more secure methods. Could you tell me where the error lies because I can't seem to fix it. – craibson Mar 06 '19 at 19:15
  • 1
    You aren't using parameters is the error. You are passing text as numbers, essentially. – LarsTech Mar 06 '19 at 19:16

1 Answers1

1
  • Always use parameterized sql and avoid string concatenation to add values to sql statements. See How can I add user-supplied input to an SQL statement?, and Exploits of a Mom. This is the main problem in your code.
  • Also wrap your connection instances in using blocks (that goes for any instance where IDisposable is implemented by the type).
  • Also password and email are reserved words in some RDBMS's. You should escape them using brackets. ([email], [password])

See your edited code, make sure you select the correct SqlDbType as well as length. I took some guesses on both.

private void enterdetails_Click(object sender, EventArgs e)
{
    string query = "INSERT INTO tbl_register([email],[password]) VALUES(@email,@password)";

    using(SqlConnection sqlconn = new SqlConnection(@""))
    using(SqlCommand comm = new SqlCommand(query, sqlconn))
    {
        sqlconn.Open();
        comm.Parameters.Add("@email", SqlDbType.VarChar, 200).Value = mail.Text
        comm.Parameters.Add("@password", SqlDbType.VarBinary, 256).Value = Hashing.ComputeHash(pass.Text,  Supported_HA.SHA256, ASCIIEncoding.ASCII.GetBytes("Supported_HA.SHA256"));

        comm.ExecuteNonQuery();
    }
}
Igor
  • 60,821
  • 10
  • 100
  • 175
  • Thanks for the help it seems like parameterized sql is very important so I will look into this. I have tried your suggestion and the old error has gone, I now get inavlid cast type can't convert string to binary. The datatypes for my db are both varbinary max so i changed email from varchar to varbinary but I am still getting the same casting error, would you have an idea as to why I would be facing this issue? – craibson Mar 06 '19 at 19:33
  • @craibson - `mail.Text` is a string and you want to insert into the column `email`. Seems to me your chose the wrong data type in your Db Schema, that should probably be varchar or nvarchar and not varbinary. – Igor Mar 06 '19 at 19:35
  • Yeah that was the issue my bad. Your help was really appreicated also the help of the other commentors. I already had a login page before I had the hashing function where the user could login and I had sql select function to check plain text pasword and email, now that I have hashing implemented will my login code be vastly different? – craibson Mar 06 '19 at 19:53
  • @craibson - No but instead of comparing the plain text passwords you will want to hash the password input in the login screen and compare that to the stored hash. Again, use parameters. If this is for an actual prod. site be sure to research hash algorithms as the SHA is not suitable for passwords. A better alternative would be any of these 3: [pbkdf2](https://en.wikipedia.org/wiki/PBKDF2), [scrypt](https://en.wikipedia.org/wiki/Scrypt), [bcrypt](https://en.wikipedia.org/wiki/Bcrypt) – Igor Mar 06 '19 at 19:58
  • @craibson - finally please consider marking an answer using the checkmark on the left side of the answer. – Igor Mar 06 '19 at 19:59