0

Somebody help me with my code. I create a edit password page containing the Current Password, New Password and Confirm Password. Here's my code:

edit_password.php

<form action="editpassword_process.php" method="post">
<table>
    <tr class="form-group has-feedback has-success">
        <td><h4>Current Password</h4></td>
        <td><div class="control-group input-lg"><input type="password" placeholder="" passfield="true" id="currentpassword" name="currentpassword"></div></td> <!-- class="input-sm" required -->
        <td><span id="messagebox"></span></td>
    </tr>
    <tr>
        <td><h4>New Password</h4></td>
        <td><div class="control-group input-lg"><input type="password" placeholder="" passfield="true" id="newpassword1" name="newpassword1"></div></td> <!-- required class="input-sm" -->
    </tr>
    <tr>
        <td><h4>Confirm Password</h4></td>
        <td><div class="control-group input-lg"><input type="password" placeholder="" passfield="true" id="newpassword2" name="newpassword2" onKeyUp="checkPass(); return false;"></div></td> <!-- required class="input-sm" -->
        <span id="confirmMessage" class="confirmMessage"></span>
    </tr>
</table>
    <button class="btn btn-info">Submit</button>
</form>

Here's my code of editpassword_process.php

<?php

include('connection.php');

$currentpw = $_POST['currentpassword'];
$newpw = $_POST['newpassword1'];
$confirmnewpw = $_POST['newpassword2'];

$res = mysql_query("SELECT user_password FROM `tbl_userlist` WHERE userid = '".$_SESSION['userid']."'");

if($currentpw != mysql_result($res, 0)){
    echo "You entered an incorrect password";
}

if($newpw = $confirmnewpw){
    $sql = mysql_query("UPDATE tbl_userlist SET user_password = '$newpw' WHERE userid = '".$_SESSION['userid']."'");
}

if($sql){
    echo "You have successfully changed your password";
}
else{
    echo "The new password and confirm pasword fields must be the same";
}

?>

When i click the submit it appears an alert that shows Validated OK but my database didn't update.

Thank you in advance

3
  • 1
    You have an sql injection vulnerability that is a much bigger problem. Commented Jan 5, 2016 at 3:32
  • 1
    Also you appear to be storing plain text passwords which is also a very big security flaw. Commented Jan 5, 2016 at 3:34
  • 2
    Most people stopped using mysql commands in favour of mysqli or pdo 7 years ago. Your script doesn't stop execution if the password is incorrect. You're assigning a variable, instead of comparing it (= vs ==). You are storing the password in plain text (a BIG no-no). crackstation.net/hashing-security.htm Commented Jan 5, 2016 at 3:35

3 Answers 3

7

There are a few things wrong with your code, but I'll start with why your user password isn't updating. You haven't started a session anywhere in your code. Anywhere you use sessions, you need to start them with:

session_start();

This should be the very first thing you do after your opening <?php tag.

You're assigning(=) and not comparing (==) in a lot of your if(){.. comparison blocks, these will evaluate to TRUE, running the condition.

Now on to the bad, you're using a deprecated library. All mysql_* functions are deprecated and removed as of PHP7. It's best to get ahead of the curve-ball by learning either of these 2 libraries:

Either of them will mitigate any SQL Injections you'd come across with your currently vulnerable code. Not to mention the fact that you're storing passwords in plain text. Imagine the implications when (not if) your database is hacked, I hope this isn't a production environment.

PHP makes it super simple to hash a password, just check out:

They'll sort out your password hashing.


To simplify what you're doing, this would be a PDO example along with hashing of your passwords to show you how simple it is to achieve what you're trying to do:

<?php
session_start();
include('connection.php');

$currentpw = $_POST['currentpassword'];
$newpw = $_POST['newpassword1'];
$confirmnewpw = $_POST['newpassword2'];

// start your PDO object
$db = new PDO('mysql:host=localhost;dbname=DATABASE', 'username','password');
$statement = $db->prepare("SELECT user_password FROM `tbl_userlist` WHERE userid = :userid");
$statement->execute(array(':userid' => $_SESSION['userid']));
// check if we have a row
if ($statement->rowCount() > 0) {
    $data = $statement->fetch(PDO::FETCH_ASSOC);
    $current_password_hash = $data['user_password'];
    // check current password is correct.
    if (!password_verify($currentpw, $current_password_hash)) {
        // wrong "current" password.
        die("You entered an incorrect password");
    }
    // check that both passwords match.
    if (trim($confirmnewpw) !== trim($newpw)) {
        // new passwords dont match
        die("The new password and confirm pasword fields must be the same");
    }
    // can only get here if passwords match.
    // so hash the new password and store in the database.
    $newpwhash = password_hash(trim($confirmnewpw), PASSWORD_BCRYPT, array('cost' => 11));
    // now lets update table to add new password hash.
    $update = $db->prepare("UPDATE tbl_userlist SET user_password = :newpwhash WHERE userid = :userid");
    if($update->execute(array(':newpwhash' => $newpwhash, ':userid' => $_SESSION['userid']))) {
        // password updated successfully.
        die("You have successfully changed your password");
    } else {
        // failed to update, check logs to ammend.
        die('Failed to update password.');
    }
} else {
    // wrong "current" password.
    die("No password found for you...");
}

Needless to say, this will mean you'll have to change your login process too, but it's simple. All you'll need to do is fetch the password and harness password_verify(), and voila, you're sorted.

(Not to mention, a ton more secure.)

Sign up to request clarification or add additional context in comments.

1 Comment

@pvegetah My pleasure mate. Glad it helped!
0

Change following things

    <?php
session_start(); // Start session as you are using it
include('connection.php');

$currentpw = $_POST['currentpassword'];
$newpw = $_POST['newpassword1'];
$confirmnewpw = $_POST['newpassword2'];

$res = mysql_query("SELECT user_password FROM `tbl_userlist` WHERE userid = '".$_SESSION['userid']."'");

if($currentpw != mysql_result($res, 0)){
    echo "You entered an incorrect password";
}

if($newpw = $confirmnewpw){ // Compare using == or ===
    $sql = mysql_query("UPDATE tbl_userlist SET user_password = '$newpw' WHERE userid = '".$_SESSION['userid']."'");
// mysql_query("UPDATE tbl_userlist SET user_password = '$newpw' WHERE userid = '".$_SESSION['userid']."'", $connectionVar);
}

if($sql){ // Here You have not executed the query now use $connection->query($sql)
// OR mysql_query($connection,$sql); any other
    echo "You have successfully changed your password";
}
else{
    echo "The new password and confirm password fields must be the same";
} ?>

2 Comments

if you have used connection var then try to echo error like this echo mysqli_error($con); check for mysql it is for mysqli connection. After checking error someone will be able to help you. so post your error
also correct that mysql_query($connectionVariable, $sqlQuery);
0

when you running this code :

"UPDATE tbl_userlist SET user_password = '$newpw' WHERE userid = '".$_SESSION['userid']."'"

that's mean you update tbl_userlist with criteria userid from session. value from session can be used if you start the session using code session_start();

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.