I cant seem to understand the issue with this statement, admittedly i have not written this code myself but have found it and modified it to work with what im doing, as most open source code is. This is for a secure login script that will pull a salted password from the database and compare it but when its executed nothing works, and an error always returns. I dont understand this because my registration works just fine so i know my connection to my database is working right and the salt function works properly as well, but my login function seems to be misbehaving. Below is what i have so far.

function login_verify($email, $password, $mysqli) {
    // Using prepared statements means that SQL injection is not possible. 
    if ($stmt = $mysqli->prepare("SELECT id, username, password, salt FROM approved_users WHERE email = ? LIMIT 1")) {
        $stmt->bind_param('s', $email);  // Bind "$email" to parameter.
        $stmt->execute();    // Execute the prepared query.
        $stmt->store_result();

        // get variables from result.
        $stmt->bind_result($user_id, $username, $db_password, $salt);
        $stmt->fetch();

        // hash the password with the unique salt.
        $password = hash('sha512', $password . $salt);
        if ($stmt->num_rows == 1) {
            // If the user exists we check if the account is locked
            // from too many login attempts 

            if (checkbrute($user_id, $mysqli) == true) {
                // Account is locked 
                // Send an email to user saying their account is locked
                return false;
            } else {
                // Check if the password in the database matches
                // the password the user submitted.
                if ($db_password == $password) {
                    // Password is correct!
                    // Get the user-agent string of the user.
                    $user_browser = $_SERVER['HTTP_USER_AGENT'];
                    // XSS protection as we might print this value
                    $user_id = preg_replace("/[^0-9]+/", "", $user_id);
                    $_SESSION['user_id'] = $user_id;
                    // XSS protection as we might print this value
                    $username = preg_replace("/[^a-zA-Z0-9_\-]+/", "", $username);
                    $_SESSION['username'] = $username;
                    $_SESSION['login_string'] = hash('sha512', 
                              $password . $user_browser);
                    // Login successful.
                    return true;
                } else {
                    // Password is not correct
                    // We record this attempt in the database
                    $now = time();
                    $mysqli->query("INSERT INTO login_attempts(user_id, time) VALUES ('$user_id', '$now')");
                    return false;
                }
            }
        } else {
            // No user exists.
            return false;
        }
    }
}

this is the script i have norrowed it down too, at least i believe it is.

oh and the check brute function is to see how many times the same user has logged in to keep a brute force attack from happening. here is the code on that

function checkbrute($user_id, $mysqli) {
    // Get timestamp of current time 
    $now = time();

    // All login attempts are counted from the past 2 hours. 
    $valid_attempts = $now - (2 * 60 * 60);

    if ($stmt = $mysqli->prepare("SELECT time FROM login_attempts WHERE user_id = ? AND time > '$valid_attempts'")) {
        $stmt->bind_param('i', $user_id);

        // Execute the prepared query. 
        $stmt->execute();
        $stmt->store_result();

        // If there have been more than 5 failed logins 
        if ($stmt->num_rows > 5) {
            return true;
        } else {
            return false;
        }
    }
}

funny this is the check brute works like a charm and i KNOW the function is being called because my check brute function updates the database with info on that login attempt.

heres where it gets odd, i know the function is being called and working due to the feedback but for some reason its not returning the desired result.

i will include all functions associated with the login script including the index itself.

here is the javascript in the form of an included file called forms.js

function formhash(form, password) {
    // Create a new element input, this will be our hashed password field. 
    var p = document.createElement("input");

    // Add the new element to our form. 
    form.appendChild(p);
    p.name = "p";
    p.type = "hidden";
    p.value = hex_sha512(password.value);

    // Make sure the plaintext password doesn't get sent. 
    password.value = "";

    // Finally submit the form. 
    form.submit();
}

function regformhash(form, uid, email, password, conf) {
     // Check each field has a value
    if (uid.value == ''         || 
          email.value == ''     || 
          password.value == ''  || 
          conf.value == '') {

        alert('You must provide all the requested details. Please try again');
        return false;
    }

    // Check the username

    re = /^\w+$/; 
    if(!re.test(form.username.value)) { 
        alert("Username must contain only letters, numbers and underscores. Please try again"); 
        form.username.focus();
        return false; 
    }

    // Check that the password is sufficiently long (min 6 chars)
    // The check is duplicated below, but this is included to give more
    // specific guidance to the user
    if (password.value.length < 6) {
        alert('Passwords must be at least 6 characters long.  Please try again');
        form.password.focus();
        return false;
    }

    // At least one number, one lowercase and one uppercase letter 
    // At least six characters 

    var re = /(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{6,}/; 
    if (!re.test(password.value)) {
        alert('Passwords must contain at least one number, one lowercase and one uppercase letter.  Please try again');
        return false;
    }

    // Check password and confirmation are the same
    if (password.value != conf.value) {
        alert('Your password and confirmation do not match. Please try again');
        form.password.focus();
        return false;
    }

    // Create a new element input, this will be our hashed password field. 
    var p = document.createElement("input");

    // Add the new element to our form. 
    form.appendChild(p);
    p.name = "p";
    p.type = "hidden";
    p.value = hex_sha512(password.value);

    // Make sure the plaintext password doesn't get sent. 
    password.value = "";
    conf.value = "";

    // Finally submit the form. 
    form.submit();
    return true;
}

here is the index.php

<?php
include_once 'includes/db_connect.php';
include_once 'includes/functions.php';

sec_session_start();

if (login_check($mysqli) == true) {
    $logged = 'in';
} else {
    $logged = 'out';
}
?>
<!DOCTYPE html>
<html>
    <head>
        <title>Secure Login: Log In</title>
        <link rel="stylesheet" href="css/styles.css" />
        <script type="text/JavaScript" src="js/sha512.js"></script> 
        <script type="text/JavaScript" src="js/forms.js"></script> 
    </head>
    <body>
        <?php
        if (isset($_GET['error'])) {
            echo '<p class="error">Error Logging In!</p>';

            if ($stmt = $mysqli->prepare("SELECT (id, username, password, salt) FROM approved_users WHERE email = ? LIMIT 1")) {
        $stmt->bind_param('s', $email);  // Bind "$email" to parameter.
        $stmt->execute();    // Execute the prepared query.
        $stmt->store_result();

        // get variables from result.
        $stmt->bind_result($user_id, $username, $db_password, $salt);
        $stmt->fetch();

        echo "<br>";
        echo $user_id;
        echo "<br>";
        echo $username;
        echo "<br>";
        echo $db_password;
        echo "<br>";
        echo $salt;
        echo "<br>";

        // hash the password with the unique salt.
        @$password = hash('sha512', $password . $salt);

        echo $password;
        echo "<br>";

    }
}
        ?> 
        <form action="includes/process_login.php" method="post" name="login_form">                      
            Email: <input type="text" name="email" />
            Password: <input type="password" name="password" id="password"/>
            <input type="button" value="Login" onclick="formhash(this.form, this.form.password);" /> 
        </form>

<?php
        if (login_check($mysqli) == true) {
                        echo '<p>Currently logged ' . $logged . ' as ' . htmlentities($_SESSION['username']) . '.</p>';

            echo '<p>Do you want to change user? <a href="includes/logout.php">Log out</a>.</p>';
        } else {
                        echo '<p>Currently logged ' . $logged . '.</p>';
                        echo "<p>If you don't have a login, please <a href='register.php'>register</a></p>";
                }
?>      
    </body>
</html>

and here is the file the index.php defaults to to call the functions

<?php
include_once 'db_connect.php';
include_once 'functions.php';

sec_session_start(); // Our custom secure way of starting a PHP session.

if (isset($_POST['email'], $_POST['p'])) {
    $email = $_POST['email'];
    $password = $_POST['p']; // The hashed password.

    if (login_verify($email, $password, $mysqli) == true) {
        // Login success 
        header('Location: ../protected_page.php');
    } else {
        // Login failed 
        header('Location: ../index.php?error=1');
    }
} else {
    // The correct POST variables were not sent to this page. 
    echo 'Invalid Request';
}
?>

i know this is a huge post but i literally have looked this code over line for line and i cannot see the issue. Thanks in advance for any and all wisdom you can throw my way.

Recommended Answers

All 8 Replies

looks like you may be hashing the password twice when when checking

once on form post and once on test

where is it being hashed twice? Also thank you for the input.

AH HA! i have finally narrowed it down to the error but im in a new predicament, the error is in my login_verify fuinction when i go to check the hashed and salted password with the one inside the database, even though i have read plenty on it and it should be working for some reason when i echo out teh variables they are different even though they are using the same salt and be dehashed by the same encryption. WHAT GIVES :/ regardless i have found my issue thanks to the little poke jstfsklh211 gave me.

the code thats nto working looks like this

if ($db_password == $p_password) {
                    $_SESSION['uhoh2'] = "the comparison of the password with the hashed one is true and makes it passed this check as well";
                    // Password is correct!
                    // Get the user-agent string of the user.
                    $user_browser = $_SERVER['HTTP_USER_AGENT'];
                    // XSS protection as we might print this value
                    $user_id = preg_replace("/[^0-9]+/", "", $user_id);
                    $_SESSION['user_id'] = $user_id;
                    // XSS protection as we might print this value
                    $username = preg_replace("/[^a-zA-Z0-9_\-]+/", "", $username);
                    $_SESSION['username'] = $username;
                    $_SESSION['login_string'] = hash('sha512', $p_password . $user_browser);
                    // Login successful.
                    return true;
                }

and the dehashing is this

$db_password = hash('sha512', $db_password.$salt);
$_SESSION['database_pass'] = $db_password;
$p_password = hash('sha512', $p_password.$salt);
$_SESSION['given_pass'] = $p_password;

// note that the session variables are just for testing

To clarify things when you were encrypting the password during registration, I hope you stored the salt which was added to the password during the encryption in the database.

i did, but i cant seem to figure out why its not dehashing correctly, i rehash the values like this

function login($email, $password, $mysqli) {
    // Using prepared statements means that SQL injection is not possible. 
    if ($stmt = $mysqli->prepare("SELECT id, username, password, salt FROM approved_users WHERE email = ? LIMIT 1")) {
        $stmt->bind_param('s', $email);  // Bind "$email" to parameter.
        $stmt->execute();    // Execute the prepared query.
        $stmt->store_result();

        // get variables from result.
        $stmt->bind_result($user_id, $username, $db_password, $salt);
        $stmt->fetch();

        // hash the password with the unique salt.
        $db_password = hash('sha512', $db_password.$salt);
        $_SESSION['database_pass'] = md5($db_password);
        $password = hash('sha512', $password.$salt);
        $_SESSION['given_pass'] = md5($password);
        if ($stmt->num_rows == 1)
        {
            $_SESSION['uhoh'] = "the statement check is true and correct";
            // If the user exists we check if the account is locked
            // from too many login attempts 

            if (checkbrute($user_id, $mysqli) == true) 
            {
                // Account is locked 
                // Send an email to user saying their account is locked
                return false;
            }
            else 
            {
                // Check if the password in the database matches
                // the password the user submitted.
                if ($db_password == $password) {
                    $_SESSION['uhoh2'] = "the comparison of the password with the hashed one is true and makes it passed this check as well";
                    // Password is correct!
                    // Get the user-agent string of the user.
                    $user_browser = $_SERVER['HTTP_USER_AGENT'];
                    // XSS protection as we might print this value
                    $user_id = preg_replace("/[^0-9]+/", "", $user_id);
                    $_SESSION['user_id'] = $user_id;
                    // XSS protection as we might print this value
                    $username = preg_replace("/[^a-zA-Z0-9_\-]+/", "", $username);
                    $_SESSION['username'] = $username;
                    $_SESSION['login_string'] = hash('sha512', $password . $user_browser);
                    // Login successful.
                    return true;
                }
                else 
                {
                    // Password is not correct
                    // We record this attempt in the database
                    $now = time();
                    $mysqli->query("INSERT INTO login_attempts(user_id, time) VALUES ('$user_id', '$now')");
                    return false;
                }
            }
        }
        else
        {
            // No user exists.
            return false;
        }
    }
}

in my code you can see that i pull the salt along with the hashed password and then try to dehash them witht he function hash() but it doest return correctly and so my comparison doesnt work at all. am i dehashing these wrong or is there something im missing?

thank you diafol, lol you always seem to be pointing me in the right direction. I have already checked into password_hash() and password_verify because i was thinking it would be easier on security if these defaults change with the php version rather than the code. Also checked your script out and wow man.. love that feeling i get when im like yeah im getting pretty good at this then i see that and im like "NOPE im still noobing pretty hard over here"

in all seriousness though i was interested in the sha512 encryption because it was the type my employer selected not that i think he would know the difference if i did go with phps simple take on it. Would there be a simple thing im doing wrong because i have gone over the logic time and time again and when i hash them (i know now its one way thank you for that knowledge) the hash i generate from using the salt i generated and saved in teh database should be the same when i apply the hash to the raw password im given, the only thing i could see that im doing wrong is hashing it one too many times somewhere but i cant see to find it. I also hash the raw input with javascript which has me wondering if that is even necessary since i sould easily just take the raw input without creating the hidden field "p" and just hashing that, clearing it and then retrieving the hashed data and doing a compare by hashing the hashed raw with my stored salt from the registration.

Also would it matter security wise if i just took the raw data hashed it with password_hash() and then sent it? would this be a security problem? or is it still better to have a javascript back end handle it instead?

AHHAHA! YES i just wanted to let everyone know that i figured out what my issue was, i was appending a newly generated salt on the end of my compared hash and then doing the comparison, all i had to do was NOT hash the stored values from the database and then just hash once the values from the given input. Then i just compared the hashes and VOILA i have my magical hashy goodness. Finally i also figured out that my checkbrute fucntion was backwards, i had it set to true always so it wouldnt even make it through to the next if statement, that was my bad. Now its working great and i have some tighter security on my program, THANK YOU EVERYONE for all of you excellent assistance!!!

commented: Glad you got it fixed +15
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.