PHP Login Not working?

[renfley]

Hey guys ive put together a super simple login and i can figure out the problem since it just wont create the sessions and echo the default error

If anyone has any idea why i am getting this issue that would be awesome

 

<?php
session_start();
// dBase file
include "../admin/config.php";


if (!$_POST["username"] || !$_POST["password"])
  {
  die("You need to provide a username and password.");
  }

// Create query
$q = "SELECT * FROM `nm_users` WHERE `username`='".$_POST["username"]."' AND `password`=PASSWORD('".$_POST["password"]."') LIMIT 1";
// Run query
$r = mysql_query($q);

if ( $obj = @mysql_fetch_object($r) )
  {
  // Login good, create session variables
  $_SESSION["valid_id"] = $obj->id;
  $_SESSION["valid_user"] = $_POST["username"];
  $_SESSION["valid_time"] = time();

  // Redirect to member page
  Header("Location: members.php");
  }
else
  {
  // Login not successful
  die("Sorry, could not log you in. Wrong login information.");
  }


?>

[The Letter E]

Hey guys ive put together a super simple login and i can figure out the problem since it just wont create the sessions and echo the default error

If anyone has any idea why i am getting this issue that would be awesome

 

<?php
session_start();
// dBase file
include "../admin/config.php";


if (!$_POST["username"] || !$_POST["password"])
  {
  die("You need to provide a username and password.");
  }

// Create query
$q = "SELECT * FROM `nm_users` WHERE `username`='".$_POST["username"]."' AND `password`=PASSWORD('".$_POST["password"]."') LIMIT 1";
// Run query
$r = mysql_query($q);

if ( $obj = @mysql_fetch_object($r) )
  {
  // Login good, create session variables
  $_SESSION["valid_id"] = $obj->id;
  $_SESSION["valid_user"] = $_POST["username"];
  $_SESSION["valid_time"] = time();

  // Redirect to member page
  Header("Location: members.php");
  }
else
  {
  // Login not successful
  die("Sorry, could not log you in. Wrong login information.");
  }


?>

 

Try:

//did your mysql object get created?
//since you have the @mysql_fetch_object, you wouldn't know the difference
var_dump($obj);

[Vel]

You've left yourself wide open for SQL Injection attacks. Before using anything a user submits in a SQL statement always escape it. Your SQL statement should be:

 

<?php
$u = mysql_real_escape_string(trim($_POST['username']));
$p = mysql_real_escape_string(trim($_POST['password']));
$q = "SELECT * FROM `nm_users` WHERE `username`='$u' AND `password`= PASSWORD('$p') LIMIT 1";

 

Also, just me but I wouldn't use LIMIT 1. I would allow as many results as possible and then use mysql_num_results to see if there are more than 1. If you use unique usernames and more than 1 result comes back you know something isn't right .

[Muddy_Funster]

to add to Vel's comment - never use select * from a user info table.  There is no reason on this planet (or any other one that you'd care to visit) that you would need or want to pull password information out of the database during login.

[Jessica]

to add to Vel's comment - never use select * from a user info table.  There is no reason on this planet (or any other one that you'd care to visit) that you would need or want to pull password information out of the database during login.

If you have your passwords encrypted, sure there is. To compare against the encrypted version of the submitted password. Because we KNOW you're not storing passwords plain text, right?

[PFMaBiSmAd]

PASSWORD('".$_POST["password"])

 

^^^ You should not use the mysql PASSWORD() function in your application (there's a note to that effect in the mysql documentation for the PASSWORD() function.)

 

You should also remove the @ in front of the @mysql_fetch_object($r) statement. If you are getting an error at the fetch statement, because the query failed due to an error of some kind, you would want to know that and then troubleshoot why the query is failing.

[Vel]

to add to Vel's comment - never use select * from a user info table.  There is no reason on this planet (or any other one that you'd care to visit) that you would need or want to pull password information out of the database during login.

If you have your passwords encrypted, sure there is. To compare against the encrypted version of the submitted password. Because we KNOW you're not storing passwords plain text, right?

No, that you do in SQL and only in SQL. Muddy is right, don't load passwords from the database.

[Jessica]

I'm just curious what problem you think there would be with loading an encyrpted password (I'm talking a one way hash).

There's many ways you can do a login, but if you're not pulling out the password you have to do at minimum 2 SQL selects (unless of course you don't want to differentiate between user not found, and user/password mismatch, which you might but users would hate). If you pull it you can limit it to one.

Obviously 1 query doesn't make a huge difference but it just seems easier to me then selecting for the username, then selecting for a match on username and password. If it's encyrpted, why does it matter?

[Vel]

I'm just curious what problem you think there would be with loading an encyrpted password (I'm talking a one way hash).

There's many ways you can do a login, but if you're not pulling out the password you have to do at minimum 2 SQL selects (unless of course you don't want to differentiate between user not found, and user/password mismatch, which you might but users would hate). If you pull it you can limit it to one.

Obviously 1 query doesn't make a huge difference but it just seems easier to me then selecting for the username, then selecting for a match on username and password. If it's encyrpted, why does it matter?

Why would you need to SQL selects? You simply encrypt the password before running any query and then compare that with the one in the database. There's no need for any second SQL select.

[Jessica]

I'm just curious what problem you think there would be with loading an encyrpted password (I'm talking a one way hash).

There's many ways you can do a login, but if you're not pulling out the password you have to do at minimum 2 SQL selects (unless of course you don't want to differentiate between user not found, and user/password mismatch, which you might but users would hate). If you pull it you can limit it to one.

Obviously 1 query doesn't make a huge difference but it just seems easier to me then selecting for the username, then selecting for a match on username and password. If it's encyrpted, why does it matter?

Why would you need to SQL selects? You simply encrypt the password before running any query and then compare that with the one in the database. There's no need for any second SQL select.

 

I used the wrong word then, Query not select. You're still hitting the DB.

[Vel]

Why would you need to SQL selects? You simply encrypt the password before running any query and then compare that with the one in the database. There's no need for any second SQL select.

 

I used the wrong word then, Query not select. You're still hitting the DB.

Whatever word, query, select, it's still 1.

[Jessica]

Can you show me an example of how you can do this? Where you can tell the difference between a mismatched password and a user not existing, and only do one query?

[Vel]

Can you show me an example of how you can do this? Where you can tell the difference between a mismatched password and a user not existing, and only do one query?

Seems I misread your post and overlooked the part where you mentioned getting the difference between a mismatched password and a user not existing. For that you're right, it would require 2 statements and I apologise for saying you were wrong. However that said, doing that is a security risk. It allows users to find out valid usernames. You shouldn't ever do that.

[Jessica]

Can you show me an example of how you can do this? Where you can tell the difference between a mismatched password and a user not existing, and only do one query?

Seems I misread your post and overlooked the part where you mentioned getting the difference between a mismatched password and a user not existing. For that you're right, it would require 2 statements. However that said, doing that is a security risk. It allows users to find out valid usernames. You shouldn't ever do that.

Of course you should! Sometimes you have to choose between security and usability.

 

Most users will use the same username on multiple websites, and sometimes have to choose a different one. For example, I use this username on almost everything. On a few websites I choose a different one for whatever reason. If I go to login as jesirose on those sites, and I get told I have the wrong password, then I have to spend time doing a reset password dance with a nonexistent account, and since you're not letting me know that my account doesn't exist, I'll end up checking multiple email accounts for a password reset link, which never arrives, and I have no idea why. eventually I'll either get frustrated and give up, or somehow remember this site uses a different password (yes, I should be using a password safe like keepass but in this example I'm the average user, not a SMART user).

If the site instead tells me that my username doesn't exist, it's a much faster process for me to determine what my correct username is.

Don't piss off your users over something so small.

[Vel]

I guess it would have to come down to a choice on what is more necessary, security or usability. It's always a battle between the two. In my experience, in commercial sites, security will pretty much always win out. Maybe I don't have as much experience as you (I don't know your background) and haven't had a project where usability trumps security.

[Jessica]

It's also a matter of how much security does the feature provide? On a site like this forum, none. You can find any number of valid usernames just by looking at the site. On every project I have worked on, you can find valid usernames without trying to login as a user. The only site that would have security through this feature would not ever display any usernames anywhere outside of being logged in. Look at sites like facebook, twitter, etc. Any site which has any public data submitted by users displays usernames. Many wordpress themes display author username, etc. Obviously there are lots of types that don't, IE a banking website. For sites like those, you could also add security back into it by limiting the number of login attempts before a lock out. That's really much more secure IMO than not allowing people to find out if their username is correct. Once you have a valid username, you still have to do a lot to get in with it if you don't know the password. I can't imagine the amount of security added by not telling a user their name is incorrect is that much.

YMMV.

[scootstah]

If I go to login as jesirose on those sites, and I get told I have the wrong password, then I have to spend time doing a reset password dance with a nonexistent account, and since you're not letting me know that my account doesn't exist, I'll end up checking multiple email accounts for a password reset link, which never arrives, and I have no idea why.

 

That just sounds like a design flaw.

 

When you reset your password you should have to enter the email on the account you have forgotten the password to. If the email doesn't exist, it should tell you as such.

[Jessica]

If I go to login as jesirose on those sites, and I get told I have the wrong password, then I have to spend time doing a reset password dance with a nonexistent account, and since you're not letting me know that my account doesn't exist, I'll end up checking multiple email accounts for a password reset link, which never arrives, and I have no idea why.

 

That just sounds like a design flaw.

 

When you reset your password you should have to enter the email on the account you have forgotten the password to. If the email doesn't exist, it should tell you as such.

 

Usually password resets are based off of account names, not email accounts. I've seen both. The email account can be frustrating to users who know they have an account but not what email they used to register with.

 

So now if we don't let them know the username they tried doesn't exist, and make them supply what email they used, you can run into the same problem I described but with email accounts. I actually have an account on a website I lost access to because they want me to tell them what email I used to register, and I have no clue because it was about 12 years ago. I know my password, and if they would just send it to the email on file it would likely get forwarded through to my new email address.

 

Again, there's a risk of shutting out your users too much - and stuff like this leads to people writing down usernames and passwords or storing that info plaintext IN their email account.

[scootstah]

Well they have to have something to know it's you. If you forgot your username AND your email, you'd probably screwed.

 

Also, passwords should never exist in emails at all.

[Jessica]

Well they have to have something to know it's you. If you forgot your username AND your email, you'd probably screwed.

 

Also, passwords should never exist in emails at all.

 

Many people have multiple email address.

I agree, but the idea of not telling a user if they have accidentally entered an invalid username in the name of security just seems like it would only lead to frustration and not actual security.

 

Agree on the passwords, I'm saying that's a reason to NOT be purposefully obtuse towards users. Users are not perfect, they are usually pretty far from it. You assume they are idiots and handle as such.

[Muddy_Funster]

@ Jesi

 

My point stands - you never return a password during a login script. If choices are between running a second query because someone has been an idiot or opening up my entire table to someone who has some skill and too much time on their hands, i'll take the hit of the query.  How much more frustrated is the user going to be when they find out that their entire account has been hijacked and the email address they registered with is now and forever more flooded with spam because I wanted to save a couple milliseconds of sever load?  Security and usability are not inversly related, as you seem to think, good security should have little impact on an end user - bad security can have a huge one.  If you do it right there is no trade-off, consider this much simplified login query:

SELECT CASE 
WHEN (SELECT uid FROM my_users AS firstCheck WHERE user_name = '$varName' AND user_password = '$encPass') THEN 'success'
WHEN (SELECT uid FROM my_users AS nameCheck WHERE user_name = '$varName') THEN 'nameOnly'
ELSE 'fail'
END AS login_Status

 

[Jessica]

Again, what does it matter if it's encryped? I'm asking seriously. Who is going to be able to see it, and even if they did what can they do with it? You would return an email address in a query, right? That's much more useful IMO than a encrypted password.

 

I don't think security and usability are inversely related, I'm talking about this one instance where someone has said it's a security risk to let a user know if they have entered an invalid password and you should NEVER do it. That's ridiculous.

 

The query you provided answers my other question, thanks.

[scootstah]

I believe they said it's a security risk to let your users know if the username is wrong.

 

Personally, I usually just go with an ambiguous "user/password combination is invalid" approach. It just makes it that much harder to brute force if you don't even know if the username is valid.

[Muddy_Funster]

Again, what does it matter if it's encryped? I'm asking seriously.

if you return the encrypted version of the password from the database, what's the point in encrypting it in the first place?

and even if they did what can they do with it?

seriously?

You would return an email address in a query, right?

not in any script directly accessable by an end user, no.

[Jessica]

I believe they said it's a security risk to let your users know if the username is wrong.

 

Personally, I usually just go with an ambiguous "user/password combination is invalid" approach. It just makes it that much harder to brute force if you don't even know if the username is valid.

Did you read my above example?'

 

Yes, I accidentally typed password where I meant username. If you look at my example of WHY it's annoying to not let them know if the username is invalid. Brute force shouldn't even be an issue, and depending on the nature of your site, usernames are already visible.