Question

I saw similar questions here but I don't think the answers apply to me, I'm sorry if they do..

Heres a sniplet of the code containing both procedures:

NOTE: The login and register works perfectly fine without the hashing element.

if (isset($_POST['username']) && ($_POST['password'])) {
    $username = trim($_POST['username']);
    $username = strtolower($username);
    $password = trim($_POST['password']);
    $salt = hash('md5', "$username");
    $password = hash('sha256', "$password"."$salt");

    $stmt = $dbh->prepare("SELECT `id` FROM `1_users` WHERE username=? AND password=? LIMIT 1");
    $stmt->bindValue(1, $username, PDO::PARAM_STR);
    $stmt->bindValue(2, $password, PDO::PARAM_STR);
    $stmt->execute();
    if ($stmt->rowCount()) {
        // Match
        $results = $stmt->fetch(PDO::FETCH_ASSOC);
        $_SESSION['id'] = $results['id'];
        $_SESSION['logged_in'] = true;
        $_SESSION['ip'] = hash('sha1', "{$_SERVER['REMOTE_ADDR']}");
        header ("location: account.php");
    }
    else {
        $error = 'Invalid username/password!';
    }
}

if (isset($_POST['register'])) {
    // check for all fields..
    if ((empty($_POST['r_username'])) || (empty($_POST['r_password'])) || (empty($_POST['re_password'])) || (empty($_POST['email']))) {
        $r_error = 'One of the fields was empty.';
    }

    $stmt = $dbh->prepare("SELECT `username` FROM `1_members` WHERE `username`=? LIMIT 1");
    $username = strtolower($_POST['r_username']);
    $username = trim($username);
    $stmt->bindValue(1, $username, PDO::PARAM_STR);
    $stmt->execute();
    $row = $stmt->rowCount();
    if ($row) {
        $r_error = 'that username is already in use';
    }
    else if (($_POST['r_password']) !== ($_POST['re_password'])) {
        $r_error = 'The passwords did not match.';
    }
    else if (strlen($_POST['r_username']) <= '3') {
        $r_error = 'username too short - needs to be 4 more charicters.';
    }
    else if (strlen($_POST['r_password']) <='5'){
        $r_error = 'password not long enough, please make it 6-255 charicters or more.';
    }
    else {
        // woohoo lets make the account 
        $password = trim($_POST['r_password']);
        $salt = hash('md5', "$username");
        $password = hash('sha256', "$password"."$salt");
        $email = trim($_POST['email']);

       $stmt = $dbh->prepare("INSERT INTO `1_members` (`username`, `password`, `email`) VALUES(?,?,?)");
       $stmt->bindValue(1,$username,PDO::PARAM_STR);
       $stmt->bindValue(2,$password,PDO::PARAM_STR);
       $stmt->bindValue(3,$email,PDO::PARAM_STR);
       $stmt->execute();
       $_SESSION['id'] = $dbh->lastInsertId(); 
       $_SESSION['logged_in'] = true;
       $_SESSION['ip'] = hash('sha1', "{$_SERVER['REMOTE_ADDR']}");

       if ($_SESSION['active_cart']) {
           header ("location: cart.php");
       }
       else {
           header ("location: account.php");
       }
    }
}

Before adding the hashing to the passwords, it functioned as normal, im unsure on what my problem is.

Debugging:

username: admin1 password: admin1


Login procedure turns password to: 927364bb72cee168bd52c45a5d131b5923e2926eb6e8f0f46d6d7e5765cc3401

Register procedure creates password as: 927364bb72cee168bd52c45a5d131b5923e2926eb6e8f0f46d6d7e5765cc3401

They match so what has gone wrong?

Also if i've got the wrong idea here or if im overlooking important security steps could you please advise on better methods.

Also, I'm not so worried about email validation and i'm fully aware there is lots of premade snipplets available for validating email and I will be getting to that at a later date.

Any CC is more than welcome.

EDIT: The script returns "invalid username/password combo".

Was it helpful?

Solution

I suspect that your database field for the hashed password is too small, the field must be able to store 64 characters.

Even if you can solve this problem, you have an unsafe scheme to store passwords (SHA256 is ways too fast for hashing passwords). Have a look at the PHP function password_hash(), it will generate a BCrypt hash and takes care of the generation of a safe salt. The salt will be part of the resulting 62 character string, so there is no need to store the salt separately. There exist also a compatibility pack for older PHP versions.

// Hash a new password for storing in the database.
// The function automatically generates a cryptographically safe salt.
$hashToStoreInDb = password_hash($password, PASSWORD_BCRYPT);

// Check if the hash of the entered login password, matches the stored hash.
// The salt and the cost factor will be extracted from $existingHashFromDb.
$isPasswordCorrect = password_verify($password, $existingHashFromDb);

This also means that you cannot verify the password directly within the SQL statement, instead read the hash from the database (by username), then call password_verify() with this hash.

OTHER TIPS

It could be, that PDOStatement::rowCount() will not return "1" while executed on a SELECT-query. Take a look here:

http://www.php.net/manual/en/pdostatement.rowcount.php

(Especially Example #2)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top