basic error with password encryption in php

Question

I want to reset user password using php. i got user's current and new password from html form . here's php script to reset password. But it always executes else part even if user enters correct password. how?any solution? i know there might be a simple error but i'm new at this and couldnt find any error.

 $uid = $_SESSION['uid'];
    $current_pass = $_POST['org_pass'];
    $new_pass = $_POST['new_pass'];

    if(isset($_POST['submit']))
    {
            $act_pass = $db_con->prepare("SELECT password FROM user WHERE u_id= ?");
            $act_pass->bindParam(1,$uid);

            $act_pass->execute();

            $actual_pass = $act_pass->fetchColumn();

            define('SALT', 'flyingrabbit');

            $typed_pass = md5(SALT.$actual_pass);

            if ($typed_pass == $current_pass)
            {
                $new_pass1 = md5(SALT . $new_pass);

                $res = $db_con->prepare("UPDATE user SET password= ? WHERE u_id=?");
                $res->bindParam(1,$new_pass1);
                $res->bindParam(2,$uid);

                $res->execute();

                 header("Location: profile.php"); 
                 exit;
            }
            else
            {


                   echo "<script type=\"text/javascript\">window.alert(\"You entered wrong password.\");window.location.href = 'profile.php';</script>";

             }

    }

i want to replace `$actual_pass` with `$new_pass`. `$current_pass` ..user enters in html form which is his password. `$actual_pass` is users password stored in db. — HungryDB, Jan 13 '14 at 15:10
You specified: if ($typed_pass == $current_pass) Are you 100% positive that this is what you want? It seems to me like you should be doing: if ($typed_pass == $actual_pass) For typed_pass, you're turning the password in the database into the hash. You should be doing this to the input, not what's in the database. — BetaBlaze, Jan 13 '14 at 15:11
Also, consider using a different encryption method, as MD5 is no longer secure: http://us3.php.net/manual/en/function.crypt.php — BetaBlaze, Jan 13 '14 at 15:15
There are so many things wrong here. First you just assume you're getting a row back. Your salt is a constant, which is insecure, and you're using md5 which is also insecure. Finally success and failed logins effectively do the exact same thing: take you to profile.php without starting a session or setting any kind of var. — Digital Chris, Jan 13 '14 at 15:20
The real solution here is to use [the built-in password hashing functions](http://www.php.net/manual/en/ref.password.php) that will give you secure randomized salts and hashes. — Digital Chris, Jan 13 '14 at 15:22
@DigitalChris thanks..for help. i will take that in account. — HungryDB, Jan 13 '14 at 15:24
@DigitalChris Note that the built-in functions are only available on php5.5+ — jeroen, Jan 13 '14 at 15:26
@jeroen good point; in which case I would refer OP to http://stackoverflow.com/questions/19103340/what-is-an-alternative-to-password-hash-for-php-5-5-5-0 — Digital Chris, Jan 13 '14 at 15:28
@DigitalChris Yep, I'm using ircmaxell's one myself one on various projects... — jeroen, Jan 13 '14 at 15:41

score 2 · Accepted Answer · edited May 23 '17 at 12:07

2

This looks wrong:

$actual_pass = $act_pass->fetchColumn();

// ...

$typed_pass = md5(SALT.$actual_pass);

if ($typed_pass == $current_pass)

You are hashing the information you got from the database which - I assume - is already hashed.

You probably want:

$actual_pass = $act_pass->fetchColumn();

// ...

$typed_pass = md5(SALT.$current_pass);

if ($typed_pass == $actual_pass)

Note that md5 is not recommended to hash passwords.

edited May 23 '17 at 12:07

Community

1
1

answered Jan 13 '14 at 15:13

jeroen

91,079
21
114
132

ohh..right. thanks. And about md5. i searched google and read 1 or 2 books about encryption and they recommend md5. how? – HungryDB Jan 13 '14 at 15:16
@HungryDB Check the link at the bottom of my answer. – jeroen Jan 13 '14 at 15:24

score 2 · Answer 2 · answered Jan 13 '14 at 15:16

2

You should compare hashed $current_pass and **$actual_pas**s.

Replace

$typed_pass = md5(SALT.$actual_pass); with $typed_pass = md5(SALT.$current_pass); $typed_pass == $current_pass with $typed_pass == $actual_pass

answered Jan 13 '14 at 15:16

MustDie1Bit

560
4
16

score 1 · Answer 3 · answered Jan 13 '14 at 15:12

1

It goes to the else statement because you compare $typed_pass == $current_pass but on the previous line you do this $typed_pass = md5(SALT.$actual_pass) you compare a hashed, salted password to a plain text password

answered Jan 13 '14 at 15:12

Pwner

791
5
16

basic error with password encryption in php

3 Answers3