1

I am get login errors when i test my script by logining under my own account. Do you think hashing passwords twice a bad practice?

I have hashed the users password twice in my website. Once, when they register and once, when they update their password in account update. Also i am using bcrypt method and cost of bcrypting is 10 on both hashings and i am on localhost server. 

    ///// this is the code in register.php page
    <?php      
    if(isset($_POST['registeruser']))  {
        session_start();
        $FName = $_POST['regfname'];
        $LName = $_POST['reglname'];
        $Email = mysqli_real_escape_string($conn, $_POST['regemail']);
        $origignalpassword = preg_replace('#[^a-z0-9_]#i', '', 
                             $_POST['regpassword']);
        $Passwordw = $_POST['confirmedpassword'];
        $infosql = "SELECT * FROM users WHERE useremail = '".$Email."'";
        $result = mysqli_query($conn,$infosql);
            if(mysqli_num_rows($result)>=1)
                   {
                    echo "Email already taken.";
                   }
                   else if(mysqli_num_rows($result) !=1 && $Passwordw == 
                   $origignalpassword) {
                   $Passwordhash = password_hash($Passwordw, 
                                   PASSWORD_BCRYPT, array('cost' => 10));
                    $sql = $conn->query("INSERT INTO users(firstname, 
                    lastname, useremail, Passwordcell) Values('{$FName}', 
                    '{$LName}','{$Email}','{$Psswordhash}')");
                        header('Location: login.php');
                    } else {
                        echo 'Please check your password:' . '<br>';
                    } 
             }
    ?>


    //// Below code is the code in my update.php page


    <?php session_start();  
        if(isset($_SESSION['user_id'])) { 
            } else {
                header('Location: login.php'); 
            }

        $user = $_SESSION['userid'];
        $myquery = "SELECT * FROM users WHERE `userid`='$user'";
        $result = mysqli_query($conn, $myquery);
        $row = mysqli_fetch_array($result, MYSQLI_BOTH);

        $_SESSION['upd_fnames'] = $row['firstname'];
        $_SESSION['upd_lnames'] = $row['Lastname'];
        $_SESSION['upd_emails'] = $row['useremail'];
        $_SESSION['upd_passwords'] = $row['Passwordcell'];
        $_SESSION['upd_phone'] = $row['phonenum'];
        $_SESSION['upd_bio'] = $row['biography'];   
    ?>
    <?php 
    if (isset($_POST['updateme'])) {
        $updfname = $_POST['upd_fnames'];
        $updlname = $_POST['upd_lnames'];
        $updemail = $_POST['upd_emails'];
        $updphone = $_POST['upd_phone'];
        $upd_pswd = $_POST['upd_passwords'];        
        $biography = $_POST['update_biography'];

        $Pswod = password_hash($upd_pswd, PASSWORD_BCRYPT, 
                 array('cost' => 10));
        $sql_input = $mysqli->query("UPDATE users SET firstname = '{$updfname}', Lastname = '{$updlname}', Phonenum = '{$updphone}', useremail = '{$updemail}', Passwordcell = '{$Pswod}', biography = '{$biography}' WHERE userid=$user");

        header('Location: Account.php');
    }  
    else 
    {

    }
?>
ramin kamal
  • 13
  • 3

1 Answers1

0

Your problem could be just a typo, in your registration script, instead of $Passwordhash you wrote:

"INSERT INTO users(..., Passwordcell) Values(...,'{$Psswordhash}')"

Nevertheless there are other problems with your code, and since you asked for advise, i would like to share my thoughts.

  1. Probably the biggest problem is, that your code is vulnerable to SQL-injection. Switch to prepared statements as soon as you can, writing code will become even easier than building the query as you did, and both MYSQLI and PDO are supporting it. This answer could give you a start.
  2. Passwords should not be sanitized. Remove the line $origignalpassword = preg_replace('#[^a-z0-9_]#i', '', $_POST['regpassword']), and just pass the input directly to the hash function password_hash($_POST['regpassword'], PASSWORD_DEFAULT). The password_hash() function works with any type of input.
  3. It is a good habit to place an exit after each redirection, otherwise the script will continue executing. header('Location: login.php', true, 303); exit;
  4. Do you really have reason to put the user info into the session? Instead of $_SESSION['upd_fnames'] = $row['firstname']; i would fetch the information on demand from the database. With fetching it from the database you can be sure that the information is actually set (is not null) and is up to date, you can avoid a state and you get a bit more REST full.
  5. Then last but not least i would recommend to follow some style rules, like starting variable names always with a small letter. You can avoid some silly typos and it makes your code more readable.
Community
  • 1
  • 1
martinstoeckli
  • 23,430
  • 6
  • 56
  • 87
  • Hey bud thanks for your answer but can you please explain the prepared Mysqli statement a little further. because I couldn't understand it. For example, with the code below how would I make sure if the email and password would be accepted if user has a combination of string, integer and characters in their email or password? `$sql = "SELECT * FROM _users WHERE useremail = ?"."and userpassword=?"; $statement = $conn->prepare($sql); $statement->bind_param('ss', $u_email, $password); $sql->execute(); $result = $sql->get_result();` thanks. – ramin kamal Aug 04 '15 at 18:00
  • @raminkamal - It is not possible to check the password with a query, because you stored only a salted hash. The necessary steps for the login script are 1) Search for password-hash by user `SELECT userpassword FROM _users WHERE useremail = ?`. 2) If the user was found, you can read the password-hash stored in the database. 3) In a last step you can verify the entered password with the found password-hash in code `$passwordIsCorrect = password_verify($password, $existingHashFromDb);`. A more indepth explanation you can find on my [homepage](http://www.martinstoeckli.ch/php/php.html#bcrypt). – martinstoeckli Aug 04 '15 at 18:41