This piece of code always says that the old password is incorrect?

Question

I'm trying to create a password changing facility in which user is asked to enter his old password, new password and confirm new password. If old password matches with database then the update of password takes place. Else a statement saying incorrect old password. But This piece of code always says that the old password is incorrect.Can someone find the bug in this code? I'm using md5 procedure to store password when a user is registered. Does it have anything to do with this? Pardon me for my lengthy code. I'm still a beginner. Thank You :)
HTML

<div class="col-sm-4">
   <h1 class="register-title">Change Password</h1>
       <form action="<?=$_SERVER['PHP_SELF']?>" method="post" class="register">
       <input type="password" name="oldpassword" class="register-input" placeholder="Enter Old Password">
       <input type="password" name="newpassword" class="register-input" placeholder="Enter New Password">
       <input type="password" name="password2" class="register-input" placeholder="Confirm New Password">
       <input type="submit" name="submit1" value="Update Password" class="register-button">
       </form>
 </div>

PHP

<?php
if (isset($_POST['submit1'])) {
    $oldpassword=$_POST['oldpassword'];
    $newpassword=$_POST['newpassword'];
    $password2=$_POST['password2'];
    //$password=md5($password);

    $connect = mysqli_connect('localhost', 'root', '', 'blood'); if (mysqli_connect_errno()) echo "Failed to connect to MySQL: " . mysqli_connect_error();
    $query12 = "select password from users WHERE uid = '$uid'";
    $query13 = "UPDATE users SET password='$newpassword' WHERE uid = '$uid'";
    $result12=mysqli_query($connect,$query12) or die('Error, query failed');
    if (mysqli_num_rows($result12) == 0) {
        echo 'Database is empty <br>';
    } elseif ($result12) {
       while ($row = mysqli_fetch_array($result12)) {
           if ($oldpassword == $row["password"]) {
               $result13=mysqli_query($connect,$query13) or die('Error, query failed');
           } else {
               echo "incorrect old password";
           }
       }
    }
}?>

Please be aware of that your code is vulnerable to SQL injection! Please use parameterised queries. Don't put code like this in production. — MyGGaN, Oct 01 '15 at 19:43
Prepared statement appears to be the term. My bad :) http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — MyGGaN, Oct 01 '15 at 19:48

score 1 · Accepted Answer · answered Oct 01 '15 at 18:02

1

If you said that you use md5 when storing password, then how do you think what value will be stored password? Hashed value of course.

So if $row["password"] has hashed value and your $_POST['oldpassword'] is not hashed, then what do you expect?

Proper way of comparing should be:

if (md5($oldpassword) == $row["password"])

And by the way, using md5 is not secure anymore.

answered Oct 01 '15 at 18:02

u_mulder

54,101
5
48
64

not secure?! Then what procedure should I use to store passwords?Where Do I Learn them? @u_mulder – lancelot Oct 01 '15 at 18:18
Saying MD5 is not secure is missing the major security problem with this code, what happens if my password is: "wowmypassword' #"? It translates to "UPDATE users SET password='wowmypassword' # WHERE uid = '$uid' (# is comment character) – beiller Oct 01 '15 at 19:56

This piece of code always says that the old password is incorrect?

1 Answers1