"Remember me" automatic login

I've implemented a "remember me" automatic login if no session has been set, but a remember me cookie is on their computer. I'm concerned about security and I'm not the best. I'm relatively new to OOP (just started a learning a week ago).

$database = new db;

if (isset($_SESSION['info'])){
  $token = $_SESSION['info']['token'];
  $loggedIn = 1;
} else {
  $loggedIn = 0;
  if (isset($_COOKIE["rememberMe"])){
    $rememberId = $_COOKIE["rememberMe"][0];
    $rememberKey = $_COOKIE["rememberMe"][1];
    if ($database->rememberMe($rememberId,$rememberKey) === "verified") {
      $loggedIn = 1;
    }
  }
}

$loggedIn is for use in JavaScript.

rememberMe function in the db class:

public function rememberMe ($userId,$rememberKey) {

    $conn = $this->connect();
    $stmt = $conn->prepare(
    "SELECT COUNT(*) as count 
     FROM sessions 
     WHERE remember_key = :remember_key AND user_id = :user_id"
    );
    $stmt->bindParam(":remember_key",$rememberKey);
    $stmt->bindParam(":user_id",$userId);
    $stmt->execute();
    $remember = $stmt->fetchAll();
    $rememberVerify = $remember[0]["count"];

    if ($rememberVerify > 0) {

      $stmt = $conn->prepare(
      "SELECT id,username,display_name,avatar FROM users WHERE id = :user_id"
      );
      $stmt->bindParam(":user_id",$userId,PDO::PARAM_INT);
      $stmt->execute();
      $userInfo = $stmt->fetchAll();

      $dbId = $userInfo[0]["id"];
      $dbUsername = $userInfo[0]["username"];
      $dbDisplayName = $userInfo[0]["display_name"];
      $dbAvatar = $userInfo[0]["avatar"];
      $token = hash('sha256',openssl_random_pseudo_bytes(50));

      $_SESSION['info'] = [
        'id'=>$dbId,
        'username'=>$dbUsername,
        'display_name'=>$dbDisplayName,
        'avatar'=>$dbAvatar,
        'token'=>$token
      ];
      return "verified";
    }
  }

When the user signs in with "remember me" checked:

```
if ($_POST['remember'] === "true"){
$rememberKey = hash('sha256',openssl_ra

Security


I'm concerned about security and I'm not the best.

You use prepared statements, secure random numbers, and didn't make any other giant mistakes (from what I can tell), so that is already very good.

Still, there are two issues:

• you definitely want to set these cookies as httpOnly (it may somewhat mitigate XSS attacks).

• you definitely want to hash the token in the database (use proper hashing such as bcrypt). Otherwise, once your database is compromised, an attacker can login as anyone that has an active remember me token. [your sha256 doesn't count here, an attacker can still just pass the value in the db]

And two minor points:

• do you need the token in the session? It doesn't seem so to me, so I wouldn't store it. It shouldn't happen, but session data may leak, eg on shared hosts, so if it's not necessary, I wouldn't store secret information in them.

• the feasibility of remote timing attacks is still somewhat disputed, but it's not that far fetched. With your code, it's highly unlikely that anything may happen, but still, ideally, your code should be secure against it, and I'm pretty sure that your SELECT isn't. If you use bcrypt as suggested, you get a timing safe string compare for free. Otherwise, see eg here (it also links to some more information on proper remember me mechanisms).

Misc

• rememberMe isn't named all that good. It's not a function used to remember me, but a function used to check if I am remembered. So isRemembered would be more fitting. Something like authenticateCookie or similar might also work.

• two spaces indentation is not enough, it makes your code hard to read. Use at least 4 spaces. If that leads to too long lines, reduce your nesting.

• you should share one connection across your code (for performance reasons).

• your rememberMe function may do a bit too much. It verifies the validity of a remember me token, but it also loads user info and saves it in the session. I would probably separate the second task into its own function.

StackExchange Code Review Q#122130, answer score: 3