0

I can't find my problem. Can anyone help me to check it. I'm new in C#.

  public void Btnchange_Click(object sender, EventArgs args)

 MySqlConnection con = new MySqlConnection("server=localhost;user id=root;persistsecurityinfo=True;database=user;password=1234");
        MySqlDataAdapter sda = new MySqlDataAdapter("select Password from user.register where Password='" + textoldpassword.Text + "'", con);
        DataTable dt = new DataTable();
        sda.Fill(dt);

        if (dt.Rows.Count.ToString() == "1")
        {
            if (textnewpassword.Text == textconfirmpassword.Text)
            {
                con.Open();
                MySqlCommand cmd = new MySqlCommand("update user.register set Password ='" + textconfirmpassword.Text + "' where Password ='" + textoldpassword.Text + "'", con);
                cmd.ExecuteNonQuery();

                con.Close();
                lblmsg.Text = "Succesfully Updated";
                lblmsg.ForeColor = Color.Green;
            }

            else
            {
                lblmsg.Text = "New password and confirm password should be same!";
            }

I expect it can update and change my password.

10
  • 1
    What do you mean by 'nothing happen'. Is there a response at all? Does the data update? Storing plain text passwords is asking for trouble. Writing SQL vulnerable code is asking for trouble (especially with plain text passwords). Commented Feb 18, 2019 at 7:54
  • 2
    @LauWeiqi Do you get an exception? If so, on what line? Have you debugged your code? If so, does it enter and successfully run the innermost if-block? You need to be more precise about what is not working. Commented Feb 18, 2019 at 7:59
  • 3
    C# isn't a language I'm strong in however I see no code associated with buttons. Please read comments carefully. If you set a breakpoint at the inner if statement, does it stop there when run? See this for sql injection prevention. See this for handing passwords more securely. Commented Feb 18, 2019 at 8:11
  • 3
    Also you're updating all users that happen to have the same password. You should also check for a matching userid in your WHERE clause. Commented Feb 18, 2019 at 8:15
  • 4
    @LauWeiqi google for Bobby Tables to understand what is wrong with this code. Best case, someone enters ';-- to delete all passwords. Worst case, someone writes '; drop table user.register;--. Don't use dynamic SQL, use parameterized queries. Don't write your own user management code either. The proper way to store passwords is to salt them, hash them at least 1000 times with a strong cryptographic algorithm and only store the hash. Frameworks like ASP.NET Core Identity already do this correctly Commented Feb 18, 2019 at 8:34

1 Answer 1

3

There are many many (mostly) minor mistakes in your code:

  • use some kind of Id fields in your sql tables
  • never do an update like you did (update the field WHERE this field is equals to...)
  • create your own class and bind the query result to this class
  • when a class implements IDisposable interface, always use the keyword 'using'
  • never ever user string concatenation in sql queries!!! SQL INJECTION!!! always use parametrized sql queries

Here's a simple example for your form. Let's suppose your user.register table has the following columns: - Id - Username - Password

Now let's create your own class (maybe right under your button click event, so it can be private this time):

private class MyUser
{
    public int Id { get; set; }
    public string Username { get; set; }
    public string Password { get; set; }
}

Then your button click event should look like this:

private void Btnchange_Click(object sender, EventArgs e) {
if (!textnewpassword.Text.Trim().Equals(textconfirmpassword.Text.Trim()))
{
    throw new ArgumentException("New password and confirm password should be same!");
}

List<MyUser> myUsers = new List<MyUser>();

using (MySqlConnection con =
    new MySqlConnection(
        "server=localhost;user id=root;persistsecurityinfo=True;database=user;password=1234"))
{
    using (MySqlCommand cmd = new MySqlCommand("select * from user.register where Username=@user and Password=@pass", con))
    {
        cmd.Parameters.AddWithValue("@user", textusername.Text.Trim());
        cmd.Parameters.AddWithValue("@pass", textoldpassword.Text.Trim());

        if (cmd.Connection.State != ConnectionState.Open) cmd.Connection.Open();

        using (MySqlDataReader dr = cmd.ExecuteReader())
        {
            while (dr.Read())
            {
                myUsers.Add(new MyUser
                {
                    Id = (int)dr["Id"],
                    Username = dr["Username"].ToString(),
                    Password = dr["Password"].ToString()
                });
            }
        }

        if (cmd.Connection.State == ConnectionState.Open) cmd.Connection.Close();
    }

    if (!myUsers.Any())
    {
        throw new ArgumentException("No users found with the given username/password pair!");
    }

    if (myUsers.Count != 1)
    {
        throw new ArgumentException("More than 1 user has the same username and password in the database!");
    }

    MyUser user = myUsers.First();
    user.Password = textnewpassword.Text.Trim();

    using (MySqlCommand cmd = new MySqlCommand("update user.register set Password=@pass where Id=@id"))
    {
        cmd.Parameters.AddWithValue("@pass", user.Password);
        cmd.Parameters.AddWithValue("@id", user.Id);

        if (cmd.Connection.State != ConnectionState.Open) cmd.Connection.Open();
        cmd.ExecuteNonQuery();
        if (cmd.Connection.State == ConnectionState.Open) cmd.Connection.Close();
    }
} }

...and so on.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.