WHERE Name='{NameInput.Text}' AND Password='{GetHashString(PasswordInput.Text)} not working

Question

I'm trying to get my LoginButton to work, it isn't really doing what I want it to do. I already have a RegisterButton which works perfectly and creates the account without any problems, but when trying to do my LoginButton it connects to the database but doesn't really check if the account exists using selectQuery and it should change WarningLabel.Text to "Wrong Name or Password". it does go through the first try and changes the WarningLabel.Text to "Welcome " + NameInput.Text;

private void LoginButton_Click(object sender, System.EventArgs e)
{
    string selectQuery = $"SELECT * FROM bank.user WHERE Name='{NameInput.Text}' AND Password='{GetHashString(PasswordInput.Text)}';";
    MySqlCommand cmd;

    connection.Open();
    cmd = new MySqlCommand(selectQuery, connection);
    try
    {
        cmd.ExecuteNonQuery();
        WarningLabel.Text = "Welcome " + NameInput.Text;
    } catch
    {
    WarningLabel.Text = "Wrong Name or Password";
    }

    connection.Close();
}

Best Regards - Nebula.exe

Why would you expect an exception if no records are selected? — HoneyBadger, Aug 09 '21 at 06:53
A Select query doesn't generate an error. You have to check if records have been returned. — Gabriel Heming, Aug 09 '21 at 06:54
Stop **right now** and read https://stackoverflow.com/questions/14376473/what-are-good-ways-to-prevent-sql-injection . — mjwills, Aug 09 '21 at 07:15

Cleptus · Accepted Answer · 2021-08-09T07:27:06.830

The ExecuteNonQuery is not intented to be used with SQL statements that return data, you should use ExecuteReader or ExecuteScalar, you can check the MySqlCommand.ExecuteReader documentation

Warning: Your code does have a SQL Injection vulnerability in this part of the SQL statement Name='{NameInput.Text}' Check this SQL Injection explanation

Usage example (from the documentation, slightly modified):

using (MySqlConnection myConnection = new MySqlConnection(connStr)) 
{
    using (MySqlCommand myCommand = new MySqlCommand(mySelectQuery, myConnection))
    {
         myConnection.Open();
         MySqlDataReader myReader = myCommand.ExecuteReader();
         while (myReader.Read())
         {
             Console.WriteLine(myReader.GetString(0));
         }
    }
}

Thanks, this helped and all treasure those links and check them out :) — Nebula.EXE, Aug 09 '21 at 07:07

Kuro Neko · Answer 2 · 2021-08-09T07:16:24.233

0

You should check if there are records returned. cmd.ExecuteNonQuery(); won't tell you if records are returned because it will just execute the query. You should use ExecuteScalar or a MySQL Data Reader ExecuteReader and track the results.

Note : Your code is prone to SQL Injections, you might want to use Parameters in your query like @name and @password.

Your Query goes something like this.

string selectQuery = $"SELECT IFNULL(COUNT(*),0) FROM bank.user WHERE Name=@name AND Password=@password;";

Then use parameters

cmd.parameters.AddWithValue(@name, NameInput.Text);
cmd.parameters.AddWithValue(@password, GetHashString(PasswordInput.Text));

Then verify if the query returns result

If cmd.ExecuteScalar() > 0
//If count is > 0 then Welcome
//Else Wrong username or password
End If

edited Aug 09 '21 at 07:16

answered Aug 09 '21 at 06:59

Kuro Neko

795
12
19

To be fair, only the Name would be vulnerable to SQL Injection, the Password column would be not, because no direct input would be used (notice the call to the `GetHashString` function) – Cleptus Aug 09 '21 at 07:04
@Cleptus Oww Yes that's what I meant, thank you for noticing. – Kuro Neko Aug 09 '21 at 07:17

score 0 · Answer 3 · answered Aug 09 '21 at 11:08

Your life, made easy:

private void LoginButton_Click(object sender, System.EventArgs e)
{
    var cmd = "SELECT * FROM bank.user WHERE Name=@name AND Password=@pw";
    
    using var da = new MySqlDataAdapter(cmd, connection);
    da.SelectCommand.Parameters.AddWithValue("@name", NameInput.Text);
    da.SelectCommand.Parameters.AddWithValue("@pw",GetHashString(PasswordInput.Text));

    var dt = new DataTable();
    da.Fill(dt);

    if(dt.Rows.Count == 0)
      WarningLabel.Text = "Wrong Name or Password";
    else
      WarningLabel.Text = $"Welcome {dt.Rows[0]["FullName"]}, your last login was at {dt.Rows[0]["LastLoginDate"]}";

}

Your life, made easier (with Dapper):

class User{
  public string Name {get;set;} //username e.g. fluffybunny666
  public string FullName {get;set;} //like John Smith
  public string Password {get;set;} //hashed
  public DateTime LastLoginDate {get;set;}
}

//or you could use a record for less finger wear
record User(string Name, string FullName, string Password, DateTime LastLoginDate);

...

  using var c = new MySqlConnection(connection):
  var u = await c.QuerySingleOrDefaultAsync(
    "SELECT * FROM bank.user WHERE Name=@N AND Password=@P", 
    new { N = NameInput.Text, P = GetHashString(PasswordInput.Text)}
  );

  if(u == default)
    WarningLabel.Text = "Wrong Name or Password";
  else
    WarningLabel.Text = $"Welcome {u.FullName}, your last login was at u.LastLoginDate";

WHERE Name='{NameInput.Text}' AND Password='{GetHashString(PasswordInput.Text)} not working

3 Answers3