log in password verification

Question

0.00/5 (No votes)

See more:

here, i m verifying my password to an another page,but it is always redirecting.I think there is something wrong...plz help me here...
my code is:

C#

protected void Button1_Click(object sender, EventArgs e)
       {
           SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString);
           con.Open();
           string cmdstr = "select count(*) from Reg where UserName='" + TextBox1.Text + "'";
           SqlCommand checkUser = new SqlCommand(cmdstr, con);
           int temp = Convert.ToInt32(checkUser.ExecuteScalar().ToString());
           if (temp == 1)
           {
               string cmdstr2 = "select Password from Reg where UserName='" + TextBox1.Text + "'";

               SqlCommand pass = new SqlCommand(cmdstr2, con);
               string password = pass.ExecuteScalar().ToString();
               con.Close();

               if (password == TextBox2.Text)

                   Session["name"] = TextBox1.Text;
                   Response.Redirect("AfterLogin.aspx?Name="+TextBox1.Text);

               }

               else
               {
                   Label2.Visible = true;
                   Label2.Text = "invalid username or password";
               }


           }
       }
   }

Posted 19-Jul-12 11:06am

symonsarwar

Updated 19-Jul-12 11:07am

Wes Aday

v2

Add a Solution

Comments

StianSandberg 19-Jul-12 17:27pm

Try your debugger.. Set a breakpoint and see what's happening

symonsarwar 19-Jul-12 17:36pm

the problem in the password verification, when username is valid it redirects whatever the password is.If username is not ok,then it shows the message(invalid username or password)

[no name] 19-Jul-12 17:41pm

Are you sure that you are not missing a "{" right after the if (password ...) statement?

[no name] 19-Jul-12 17:39pm

Not really an answer but you are querying the DB twice when once would do. Use parameterized queries instead of string concatenation.

barneyman 19-Jul-12 18:31pm

and for the love of cheese, DO NOT store passwords, store hashes of passwords

StianSandberg 19-Jul-12 18:55pm

wish I could give your comment a 5! :)

DrWheetos 19-Jul-12 18:48pm

Don't use the username and password without sanitizing it first to remove characters deemed to be invalid. This code is so unbelievably insecure. It would be trivial to do a sql injection attack on it. Go google it if you don't know what that means. And as barneyman says, hash the password before comparing it against the database. I cringe when I see this stuff.

StianSandberg 19-Jul-12 18:54pm

Could not agree more DrWheetos! I see a lot of people asking about help with their database-code and surprisingly many have insecure sql

StianSandberg 24-Jul-12 7:08am

After answering this question i wrote an article about a few security problems with your code. Sql-injections and password storage.
You should really read it: Beginners guide to a secure way of storing passwords

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

StianSandberg · Accepted Answer · 2012-07-19T12:32:00

Wes is right! if the passwords match the session is set because that is the first line of code after the "if(passord==....)" But the ResponseRedirect is hit every time.

try to

C#

if (password == TextBox2.Text)
{ 
  Session["name"] = TextBox1.Text;
  Response.Redirect("AfterLogin.aspx?Name="+TextBox1.Text);
}

If you had attached the debugger you would have seen this behavior. You would probably seen it if your code was a little cleaner also. Hit CTRL+K D to format your code in VS.

Another thing with your code is that you are doing 2 ExecuteScalar(). You only need one. I have rewritten your code in a slightly cleaner way:

C#

protected void Button1_Click(object sender, EventArgs e)
{
    // I always user variables. Then I don't have refer to textboxes each time.
    string username = Textbox1.Text;
    string password = TextBox2.Text;

    SqlConnection conn = new SqlConnection("ConnectionString");
            
    string sql = "select password from Reg where UserName=@Username";
    SqlCommand cmd = new SqlCommand(sql, conn);

    // use sql parameter to avoid sql injection!
    cmd.Parameters.AddWithValue("@Username", username);
            
    //Open the connection as close to any other db-stuff.
    // don't keep it open unless you have to use it
    conn.Open();

    // ExecuteScalar() (which is an object) can be null!
    // using Convert.ToString() prevents any Null Reference Exception.
    // Convert.ToString(null) returns string.Empty
    string pwdFromDb = Convert.ToString(cmd.ExecuteScalar());

    // All database stuff is ok. 
    cmd.Dispose();
    conn.Close(); // Always close DB connection when done!


    // Now we can do the redirect logic
    if(pwdFromDb == password) // this will make your pwd case sensitive
    {
        // user is found and passwords are equal
        // Set session and redirect user
        Session["name"] = username;
        Response.Redirect("AfterLogin.aspx?Name=" + username);
    }
    else 
    {
        // userFoundAndPwdIsOk is false..
        Label2.Visible = true;
        Label2.Text = "invalid username or password";
    }
}

A few tips for you:
1) Always use command parameters when sending variables in your sql!
2) Use brackets around your if's if(){...}
3) Never store passwords as clear text. Hash it and use a password salt.

C#

for (int i = 4; i < 1000; i++)
{
    Console.WriteLine(i + ") ALWAYS use command parameters!");
}