Resolved login successfully for not stored data

maddyrafi

New member
Joined
Mar 19, 2022
Messages
4
Programming Experience
Beginner
I have stored userid and password in mysql database, this query is showing login successfully for not stored data, so please correct it
C#:
if (textBox9.Text != "" && textBox10.Text != "")
{
    string connectionString;
    MySqlConnection cnn;
    connectionString = @"Data Source=localhost;Initial Catalog=testDB;User ID=root;Password=mysql";
    cnn = new MySqlConnection(connectionString);
    string id = textBox9.Text;
    string password = textBox10.Text;
    textBox9.Text = "";
    textBox10.Text = "";
    string query = "select * from login where userid=@userid and password=@password";
    using (MySqlCommand cmd = new MySqlCommand(query))
    {
        cmd.Parameters.AddWithValue("@userid", id);
        cmd.Parameters.AddWithValue("@password", password);
        cmd.Connection = cnn;
        cnn.Open();
        cmd.ExecuteNonQuery();
        DialogResult dr = MessageBox.Show("Are you sure to Login now?", "Confirmation Message", MessageBoxButtons.YesNo);
        if (dr == DialogResult.Yes)
        {
            MessageBox.Show("Login Successfully");
            cnn.Close();
            this.Hide();
            Form2 f2 = new Form2();
            f2.ShowDialog();
        }
        else if (dr == DialogResult.No)
        {
            MessageBox.Show("Please Enter Correct Login details");
        }
    }
}
else
{
    MessageBox.Show("Please Enter With Correct Login Details");
}
 
As for the issue, why would you call a method named ExecuteNonQuery to execute a query? If you have read the documentation for that method or simply paid attention to Intellisense, then you know that it is for executing SQL statements that do not produce a result set, i.e. that do not retrieve any data. If you want to retrieve data then you call ExecuteReader or, for a single value, ExecuteScalar.

Also, what's the point of asking the user to confirm that they want to log in when you've already executed the query? Surely you want to get the confirmation first and only execute the query if they say "yes".
 
And actually you have a major error, and one big error, on top of your small error.

The major error is storing the password unhashed in the database. This is a major security design error. Always store salted and hashed passwords in the database. That way if your database gets stolen, hackers will have to work hard to try to find matching passwords for the usernames.

The big error is using SELECT * for a query where all you are looking for either success or failure for a match. By using SELECT * you are asking the database to return all the columns of data for that matching row -- columns that you will likely discard if all you are planning on doing is validate that they are allowed to log in. You should use SELECT COUNT(*) to instead just let the database return a count of how many matching rows there are. With that you should use ExecuteScalar().
 
Back
Top Bottom