views:

146

answers:

3

Im making a login/logout class that logs users in, sets cookies based on user's choice. The user enters their email/password and it checks the database, email/password combo exists a session is created, and a cookie is set (with the users id) and the user is redirected... I then have a function that logs users in by taking the user id saved in that cookie, checking whether that user id exists and then saving the users data in a session yet again... i was wondering if anybody see's anything potentialy wrong/unsafe about this.

Short Example, im sure you guys can get the gist of it...

function login($email, $password, $remember){
  // Check the database for email/password combo
  if(/*user exists*/){ // if the user exists
    $_SESSION = /*User data*/ // save the users data in a session
    if($remember){
      setcookie('user_id', /*User id*/); // save the user id in a cookie
    }
    header("location: index.php");// redirect
  }
}

function Check_Cookie(){
  if(isset($_COOKIE['user_id'])){
    return $this->Log_In_ID($_COOKIE['user_id']);
  }else{
    return false
  }
}

function Log_In_ID($id){
  //Check the database if the user id exists
  if(/*user exists*/){ // if the user exists
    $_SESSION = /*User data*/ // save the users data in a session
    header("location: index.php");// redirect
  }else{
    return false;
  }
}

Its not a detailed example of what im trying to ask, but im sure you can get the gist of it... Does anybody see anything potentially wrong with this. If you guys have any recommendations id love to hear them...also, do you guys use oop to log users in, or any other ways.

+6  A: 

If your user ID is a sequential number, this is pretty insecure as anyone can just change their cookie to another reasonable-looking number based on their own (e.g. if mine is 1274, I could try some other numbers in that range) and immediately spoof that user.

You would be better off assigning a temporary ID associated with that user, like a GUID. Since GUIDs are astronomically unique and practically collision-proof, they're also virtually impossible to guess or predict from outside the system.

When the user logs in, you create a new GUID and store that with the user:

UserID        TokenID                                        Expires
1274          {3F2504E0-4F89-11D3-9A0C-0305E82C3301}         9/25/2009 12:00:00

When a user returns, look up their user ID by the token, make sure the token hasn't expired and log them in. Then change their token. This secures you against the following:

  • An attacker cannot guess another user's token and spoof them
  • Token expiration cannot be circumvented by ignoring the cookie's expiration date
  • Since tokens change constantly, even if an attacker does manage to gain access to a user's cookie, the window of opportunity to take over is very small.
Rex M
In addition to this, you can also check the user's IP address is in the same range and browser string to make sure it's the same computer.
Milan Babuškov
@Milan good advice - also good things to check, though IMO slightly outside the scope of this particular question.
Rex M
Good solution, Rex. Another option is to just rely on the cookie'd sessionid -- assuming you have control and understanding of session settings. There's no hard reason to not set long session lifetimes.Your solution is still superior, since you're switching tokens -- though you could set a last-access cookie on every request, and regenerate the user's sessionid if they've been "away" long enough.
timdev
A: 

Cookies are very easily altered- so if you are only storing the ID a user could change it. By guessing they could access other accounts and potentially even get admin access. Generally I store ID along with a token when I use cookies for authentication.

You can take for example a hash of the ID and a salt value and store it- You can then verify the token when the user connects. It isn't perfect and there are more considerations for a high security site- but for a standard site that should be a good start.

Another tactic is to store a long unique session ID and use that to log the user back in.

apocalypse9
+1  A: 

You should not trust the cookie data. What happens if I edit my cookie and set my ID to say, "1" (which is likely to be an administrative user)?

Basically, don't do this.

If you want a "remember me" type function, just save a username in a cookie so you can prepopulate the login form when the user returns -- but force them to reauthenticate.

timdev
Clearly not all sites are this draconian and still manage to be secure enough. I think the question is how to achieve this level of ease on the user.
Rex M