3

I have created a forgotten password change php function for my login system that sends an email to the user that has forgotten his/her password and gives a link utilizing a hashed token in order to change their password. Once email link is selected by the user, they are then able to change their password, which then updates mysql with their new hashed password.

Every aspect of the code seems to work properly until I try to login with the new password. I receive echo that "username/password combination incorrect" (found on LOGIN.PHP page). Trying the original password echos the same error as well on the LOGIN.PHP page.

Not exactly sure why my sql query is not matching the updated password with the existing username and allowing the login?

For ease of parsing thru the code, I have excluded parts I do not believe to be the issue. I have included 5 php files.

FORGOTPASSWORD.PHP

<?php
if (!isset($_GET['email'])) {
    echo '<form action="">//Form for password reset here</form>';
    exit();
}


define('DB_USER', '');
define('DB_PASS', '');
define('DB_NAME', '');

$email = $_GET['email'];

function connect()
{
//Connect to db
}
connect();

$q = "SELECT email FROM users WHERE LCASE(TRIM(email))='" . strtolower(trim($email)) . "'";
$r = mysql_query($q);
$n = mysql_num_rows($r);

if ($n == 0) {
    echo "Email id is not registered";
    die();

}

//token updated into sql db for user
    $token=getRandomString(10);
    $q="UPDATE users SET token=('".$token."') WHERE email=('".$email."')";
    mysql_query($q);

function getRandomString($length) 
   {

//token created
}
//email creation code here

RESET.PHP

<?php
session_start();

//Define db connection parameters

    $token=$_GET['token'];

function connect() {

//Connection to db executed
    }

connect();

if(!isset($_POST['password'])){
    $q="SELECT email FROM users WHERE token='".$token."' and used='0'";
    $r=mysql_query($q);
    while($row=mysql_fetch_array($r))
   {

$email=$row['email'];
   }

If ($email!=''){
      $_SESSION['email']=$email;
}

    else die("Invalid link or Password already changed");}

$password=$_POST['password'];
$email=$_SESSION['email'];

if(!isset($password)){

echo '
//Change password form
';}

if(isset($_POST['password'])&&isset($_SESSION['email']))
{

//Update sql db with newly created password
$q="UPDATE users SET password='".md5($password)."' WHERE email='".$email."'";
$r=mysql_query($q);

if($r)mysql_query("UPDATE users SET used='1' WHERE token='".$token."'");echo "Your password is changed successfully";
if(!$r)echo "An error occurred";
}
?>

LOGIN.PHP (displays the error messsage)

<div id="loginContainer">
<div id="loginMessage">
        <?php if ( $logged == 'invalid' ) : ?>
            <p class="name_pass">
                The username/password combination is incorrect. Try again.
            </p>
        <?php endif; ?>
        <?php if ( $_GET['reg'] == 'true' ) : ?>
            <p class="success">Your registration was successful, please login below.
            </p>
        <?php endif; ?>
        <?php if ( $_GET['action'] == 'logout' ) : ?>
            <?php if ( $loggedout == true ) : ?>
                <p class="log_out">You have been successfully logged out.
                </p>
            <?php else: ?>
                <p class="problem">There was a problem logging you out.
                </p>
            <?php endif; ?>
        <?php endif; ?>
        <?php if ( $_GET['msg'] == 'login' ) : ?>
            <p class="must_login">You must login to view this content. Please login below.
                </p>
        <?php endif; ?>
</div>

CLASS.PHP (login function)

        function login($redirect) {
        global $jdb;

        if ( !empty ( $_POST ) ) {


            $values = $jdb->clean($_POST);


            $subname = $values['username'];
            $subpass = $values['password'];


            $table = 'users';


            $sql = "SELECT * FROM $table WHERE username = '" . $subname . "'";
            $results = $jdb->select($sql);


            if (!$results) {
                die('Sorry, that username does not exist!');
            }


            $results = mysql_fetch_assoc( $results );


            $storeg = $results['date'];


            $stopass = $results['password'];


            $nonce = md5('registration-' . $subname . $storeg . NONCE_SALT);


            $subpass = $jdb->hash_password($subpass, $nonce);


            if ( $subpass == $stopass ) {


                $authnonce = md5('cookie-' . $subname . $storeg . AUTH_SALT);
                $authID = $jdb->hash_password($subpass, $authnonce);


                setcookie('logauth[user]', $subname, 0, '', '', '', true);
                setcookie('logauth[authID]', $authID, 0, '', '', '', true);


                $url = "http" . ((!empty($_SERVER['HTTPS'])) ? "s" : "") . "://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'];
                $redirect = str_replace('login.php', $redirect, $url);


                header("Location: $redirect");
                exit;   
            } else {
                return 'invalid';
            }
        } else {
            return 'empty';
        }
    }

INDEX.PHP (landing page following successful login)

<?php
require_once('load.php');
$logged = $j->checkLogin();

if ( $logged == false ) {
    //Build our redirect
    $url = "http" . ((!empty($_SERVER['HTTPS'])) ? "s" : "") . "://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'];
    $redirect = str_replace('index.php', 'login.php', $url);

    //Redirect to the home page
    header("Location: $redirect?msg=login");
    exit;
} else {
    //Grab our authorization cookie array
    $cookie = $_COOKIE['logauth'];

    //Set our user and authID variables
    $user = $cookie['user'];
    $authID = $cookie['authID'];

    //Query the database for the selected user
    $table = 'users';
    $sql = "SELECT * FROM $table WHERE username = '" . $user . "'";
    $results = $jdb->select($sql);

    //Kill the script if the submitted username doesn't exit
    if (!$results) {
        die('Sorry, that username does not exist!');
    }

    //Fetch our results into an associative array
    $results = mysql_fetch_assoc( $results );
?>

Thanks for any and all help. I appreciate anyone willing to take a look at the code to help me figure out this last part to my login process. Thanks.

Michael Philibin
  • 373
  • 1
  • 2
  • 16
  • Try checking the change in entries of the database when you run your update query in Forgot Password. Do the existing value change? – devDeejay Apr 23 '17 at 01:54
  • Every time you use [the `mysql_`](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) database extension in new code **[a Kitten is strangled somewhere in the world](http://2.bp.blogspot.com/-zCT6jizimfI/UjJ5UTb_BeI/AAAAAAAACgg/AS6XCd6aNdg/s1600/luna_getting_strangled.jpg)** it is deprecated and has been for years and is gone for ever in PHP7. If you are just learning PHP, spend your energies learning the `PDO` or `mysqli` database extensions. [Start here](http://php.net/manual/en/book.pdo.php) – icecub Apr 23 '17 at 01:56
  • Dang, never liked cats anyway!Anyway, am learning PHP so I will check out the link for mysqli. – Michael Philibin Apr 23 '17 at 01:57
  • @DeeJay - As for the change in entries, the dB does reflect a change in password was made. I notice the password hashes changing. – Michael Philibin Apr 23 '17 at 01:58
  • 1
    Please do. Because as it is right now, it would take me less than 2 minutes to hack your database and do whatever I want with it. It's called: [SQL injection](https://en.wikipedia.org/wiki/SQL_injection) – icecub Apr 23 '17 at 01:59
  • Familiar with SQL injection. Wanted to learn this way before I change over and learn prepared statements and even the PDO or mysqli that you have referenced. – Michael Philibin Apr 23 '17 at 02:01
  • @MichaelPhilibin Great Lynda Courses for both of your needs. – devDeejay Apr 23 '17 at 02:03
  • @MichaelPhilibin did it work when the user just registers and tries to login? – devDeejay Apr 23 '17 at 02:04
  • I would suggest learning PDO. It's easier to understand than MySQLi and it supports most database drivers (unlike mysqli). Don't feel intimidated by it. I could teach it to you within an hour. Probably faster :) – icecub Apr 23 '17 at 02:04
  • user registration and login works flawlessly. – Michael Philibin Apr 23 '17 at 02:09
  • @DeeJay - the email password change functionality works as well and i visualize a change in the database but the original password and new (changed) password do not allow login to index.php – Michael Philibin Apr 23 '17 at 02:10
  • 1
    And you should also read about [Safe Password Hashing](http://php.net/manual/en/faq.passwords.php) on the official PHP website. Because md5 is not suitable for passwords. Not even with SALT. All in all there's a lot you still need to learn. I don't mind spending some time with you if you're serious about learning, but that's up to you – icecub Apr 23 '17 at 02:11
  • 1
    And yes, I know you just want a solution for your problem. But as I'd like to say: Don't try to swim in the deep ocean without getting your swimming certificates first. You'll simply drawn. We will fix your problem along the way :P – icecub Apr 23 '17 at 02:18
  • Try echoing out the two hashed values of LOGIN password and the STORED HASHED password. – devDeejay Apr 23 '17 at 02:22
  • @icecub feel in over my head at the moment. learning by way of finding and fixing code based on an application i am interested in building. aware i have a lot to learn and the password hashing is another one of them. would be interested in taking you up on your offer. – Michael Philibin Apr 23 '17 at 02:24
  • I've send you an invitation to start a chat – icecub Apr 23 '17 at 02:36

1 Answers1

0

In your reset.php file, you store an unsalted MD5 of the password...

$password=$_POST['password'];
$q="UPDATE users SET password='".md5($password)."' WHERE email='".$email."'";

...but then you compare it to something different...

$subpass = $values['password']; // seems to be the entered password
...
$stopass = $results['password']; // seems to be the stored MD5 password hash
...
$subpass = $jdb->hash_password($subpass, $nonce); // probably not an unsalted MD5
if ( $subpass == $stopass ) // comparison of different things

Even if you get this code to work, it will be a very unsafe solution. Instead you should absolutely switch to the password hash functions built in in PHP. The DB field should be varchar(255), the code could then look something like this:

// Hash a new password for storing in the database.
// The function automatically generates a cryptographically safe salt.
$hashToStoreInDb = password_hash($_POST['password'], PASSWORD_DEFAULT);
$q="UPDATE users SET password='".$hashToStoreInDb."' WHERE email='".$email."'";

To check the password you need to get the hash from the database and compare it with password_verify, so the comparison will be done with the same salt as was used to store the hash.

// Check if the hash of the entered login password, matches the stored hash.
// The salt and the cost factor will be extracted from $stopass.
$subpass = $values['password'];
...
$stopass = $results['password'];
...
$isPasswordCorrect = password_verify($subpass, $stopass);
if ($isPasswordCorrect)

Another problem in your code is SQL-injection, you could avoid this vulnerablity with using prepared statements. An example with MySqli or PDO you can find in another answer.

Community
  • 1
  • 1
martinstoeckli
  • 23,430
  • 6
  • 56
  • 87