HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Login function in PHP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
phploginfunction

Problem

I've been working on a simple login script. The registration part is still to be coded, but I'm done the login part. Are there any security holes in my code? I'm using SHA-512 right now but I think I'll change it to mcrypt later.


Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

    public function Login($user_email, $user_pass, $remember = false)
    {
        $user_pass = sha1($user_pass);
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if($user->password == $user_pass) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', uniqid()), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }

    }
}

Solution

I'm using SHA-512 right now but I think I'll change it to mcrypt later.

Neither of those are the proper tool for the job. Mcrypt is meant for symmetric encryption (AES, TwoFish, DES, etc.), not password hashing.

If you're running PHP 5.5+, use password_hash() and password_verify()

Otherwise, https://github.com/ircmaxell/password_compat is the way to go.

EDIT: Example code:

db = $db;
    }

public function Login($user_email, $user_pass, $remember = false)
{
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if (password_verify($user_pass, $user->password)) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', mcrypt_create_iv(64, MCRYPT_DEV_URANDOM)), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }
    }
}


When you insert a row in the DB, you want to use password_hash($user_password)

Code Snippets

<?php
Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

public function Login($user_email, $user_pass, $remember = false)
{
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if (password_verify($user_pass, $user->password)) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', mcrypt_create_iv(64, MCRYPT_DEV_URANDOM)), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }
    }
}

Context

StackExchange Code Review Q#59148, answer score: 3

Revisions (0)

No revisions yet.