-1

I'm trying to make a login webpage, and for some reason its always gets into the "false" line where login and password are incorrect. I checked in the database to see maybe I'm a douche, but this is completely true.

This is my code :

con = new SqlConnection("Data Source=MICROSOF-58B8A5\\SQL_SERVER_R2;Initial Catalog=Daniel;Integrated Security=True");
string query = "SELECT Username, Password FROM Users WHERE Username='" + Username + "' AND Password='" + Password + "'";
con.Open();
DataSet ds = new DataSet();
SqlDataAdapter adap = new SqlDataAdapter(query, con);
adap.Fill(ds);
int count = ds.Tables[0].Rows.Count;
if (count == 0)
{
    ErrorMessege.Text = "Username or Password was Incorrect!";
}
else
{
    Session["Session"] = UsernameLogin.Text;
    Response.Redirect("MenuHome.aspx");
}
con.Close();

Thanks for the helpers!

4
  • 5
    Use SqlParameters to pass the UserName and Password values to the query! Your current implementation is asking for SQL injection! (for example, try entering x' and 1=1 -- as the user name and some random value for the password to see what happens) Jan 8, 2012 at 11:01
  • 1
    Why not just use the built in asp.net sql membership provider? Jan 8, 2012 at 11:09
  • 3
    I'd like to be user 'x';DROP TABLE Users;
    – spender
    Jan 8, 2012 at 11:12
  • 5
    Storing passwords in the clear? I wouldn't want to subscribe to your login system. What's wrong with the default AspNet MembershipProvider? Rolling your own login system seems like a big waste of time and a great way to introduce security issues. A cursory glance at your code reveals 2 serious problems (at least)
    – spender
    Jan 8, 2012 at 11:14

5 Answers 5

2

Your code looks like it should work. Have you stepped through it and added a watch on ds to see what values it has? I executed the code on my machine against one of my test DBs and it worked fine.

I would however suggest something similair to this as it is a lot cleaner in my opinion:

using(SqlConnection con = new SqlConnection("Data Source=MICROSOF-58B8A5\\SQL_SERVER_R2;Initial Catalog=Daniel;Integrated Security=True"))
{
    string query = "SELECT TOP 1 Username FROM Users WHERE Username=@UserName AND Password=@Password";

    using (SqlCommand command = new SqlCommand(query, con))
    {
        command.Parameters.AddWithValue("@UserName", UserName);
        command.Parameters.AddWithValue("@Password", Password);
        con.Open();
        string username = (string)command.ExecuteScalar(); //Add Null Check
        // Do stuff if username exists         
    }
}

You should always add a using statement on an object that implements IDisposable.(SqlConnection, SqlCommand) Also to prevent SqlInjection and the likes, use parameterized queries.

1
  • 2
    He still needs to fix is password storage. You fixed the sql-injection, but not that he stores plain text passwords. Jan 8, 2012 at 11:29
0

The easiest way to check for passwords is by just creating an SqlDataReader created from

SqlCommand sqlCommand = new SqlCommand(queryString, connection);
var reader = sqlCommand.ExecuteDataReader();
if(reader.Read()){
 //you got a correct password, read fields from reader
   string username = reader[0];
}

ExecuteReader SqlDataReader

0

Have you tried to Response.Write the query string and then paste the result into a new query in the database explorer? You might be surprised :)

0

Few things to check:

  1. case (upper/lower) of the username/password value you are sending; probably due to case insensitivity it's not matching anything.

  2. Are you storing password in plain text format ... generally password would be stored encrypted ... So I would suggest you to search for username only like

    SELECT Username, Password FROM Users WHERE lower(Username) ='Username';

1
  • Nitpick: You don't encrypt passwords, you hash them. Jan 8, 2012 at 11:19
0

Ensure that the username and password validation is not case-sensitive or validate it with the correct case. If the DB uses case-sensitive collation, it may not return what you expecting.

1
  • I ensured that its written just like in the database.
    – thormayer
    Jan 8, 2012 at 11:11

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.