Reset Email Password Code by Email

[justlukeyou]

Hi,

 

I am trying use the following code to retrive a password to a registration system but I am struggling to get it to work.

 

Table Name:  organisermembers

 

row:

companyname:    testcompany

userid:                1

emailaddress:      test@example.com

password:          password

password1:        password1

 

The idea is that someone enters their email address and they receive an email to reset their password.  Can anyone please point out any issues which may stop it from working?

 

<?php


if($_GET['m'] == "link") {
    $query = mysql_query("SELECT * FROM organisermembers WHERE userid = '".mysql_real_escape_string($_GET['i'])."' LIMIT 1");
    if(mysql_num_rows($query) > 0) {
        $row = mysql_fetch_array($query);
        $r = md5($row['companyname'].$row['password']);
        if($r == $_GET['r']) {
            if($_POST['submit']) {
                $password = trim($_POST['password']);
                if(!isset($password) || empty($password)) {
                    echo "Your password could not be changed because it was empty.";
                } else {
                    mysql_query("UPDATE organisermembers SET password = '".mysql_real_escape_string(md5($password))."' WHERE userid = '".$row['userid']."'");
                    echo "Your password has been changed. You can now <a href=\"login.php\">Login</a>.";
                }
            } else {
                echo "<form action=\"forgot.php?m=link&i=".$_GET['i']."&r=".$_GET['r']."\" method=\"post\"><label for=\"password\">New Password:</label> <input type=\"password\" name=\"password\" /><br /><br /><input type=\"submit\" name=\"submit\" value=\"Save\" /></form>";
            }
        } else {
            echo "The link that you followed was invalid. Please try copying and pasting the link into your address bar manually.";
        }
    } else {
        echo "The link that you followed was invalid. Please try copying and pasting the link into your address bar manually.";
    }
    exit;
}

if($_POST['submit']) {
    $emailaddress = mysql_real_escape_string($_POST['emailaddress']);
    $query = mysql_query("SELECT * FROM organisermembers WHERE emailaddress = '".$emailaddress."' LIMIT 1");
    if(mysql_num_rows($query) > 0) {
        $row = mysql_fetch_array($query);
        $message = "Hello ".$row['companyname'].",\r\n\r\nPlease follow this link to reset your password: ".$website['url']."/siteinfo/forgot.php?m=link&i=".$row['userid']."&r=".md5($row['companyname'].$row['password'])."\r\n\r\nThanks,\r\nJohn Doe";
        $headers = "From: ".$website['companyname']." <".$website['emailaddress'].">\r\n";
        if(mail($_POST['emailaddress'], "Password Reset", $message, $headers)) {
            $success = "Instructions for resetting your password have been emailed to you.";
        } else {
            $error = "There was a problem sending the email. Please try again.";
        }
    } else {
        $error = "Invalid email address entered.";
    }
}

?>

[Psycho]

What problems are you experiencing? Errors anything?

[justlukeyou]

Hi,

 

It gives this error "Invalid email address entered."

 

But I am entering the email address in the database.

[Psycho]

After looking at the code a little closer some things don't make sense.

 

if($_GET['m'] == "link")
{
    $query = mysql_query("SELECT * FROM organisermembers WHERE userid = '".mysql_real_escape_string($_GET['i'])."' LIMIT 1");

 

You first check if the variable 'm' was set to a particular value then use the value for the variable 'i' in your query. You should really use more descriptive variable names because I don't see why you don't just check if 'i' is set.

 

mysql_query("UPDATE organisermembers SET password = '".mysql_real_escape_string(md5($password))."' WHERE userid = '".$row['userid']."'");

 

It's useless to use mysql_real_escape_string() on a hash.

 

        $r = md5($row['companyname'].$row['password']);
        if($r == $_GET['r']) {

 

You are taking two values from the database, hashing them, and then seeing if they match a value that was sent in the POST data. Is that a hidden field value that you set? Because I can't believe a user would know what that hash value should be.

 

                $password = trim($_POST['password']);
                if(!isset($password) || empty($password)) {

 

Why are you checking if $password is set right after you just set it? You should be checking if the POST value was set.

 

There's a lot of logic that I cannot make sense of because I don't know what the variables represent or what they would be expected to contain. It could be that the values you think are being passed are not what you expect or don't exist.

 

I would suggest changing the logic from

if(!error_condition_1)
{    
    if(!error_condition_2) {
        //Do somthing
    } else {
        //error message 2
    }
} else {
    //error message 1
}

 

To something like:

if(error_condition_1)
{    
    //error message 1
}
elseif(error_condition_2) {
   //error message 2
} else {
     //Do something    
}

 

Makes it much easier to follow

[Psycho]

It gives this error "Invalid email address entered."

 

But I am entering the email address in the database.

 

OK, so what values are entered into all of your variables and which branch of logic would you expect to occur? You have a lot of condition checks in there, so without knowing what the incoming values are I can't say what it should or shouldn't do. But, if that message is displayed that tells me that the very first condition if($_GET['m'] == "link") is not met, because the block of code associated with that condition ends with an exit command.

[Christian F.]

I too can only echo what Psycho posted above: You need to clean up the script a bit, and use logical and descriptive names for your variables.

Also, check for error conditions, not success, and then use a principle called "Exit early". Which means that you should not be afraid to terminate the code early, if you encounter a situation that makes it impossible for the following code to run. Since this seems to be an included file, you should not be afraid to use return in addition to the restructuring tip that Psycho gave you above.

 

In closing, and the real reason I wrote this post, I strongly recommend that you read this article about secure login systems. You're doing some really bad stuff here with your password storage, which leaves your users password quite open to attackers. A straight up MD5 hash without any salts is only marginally better than storing it as plain text, and that's a very slim margin.

[justlukeyou]

Thanks guys,

 

Where can I actually get a full membership script from? I paid someone to write this over 2 years.  Only about $40.00 but he said it would work fine.

[PFMaBiSmAd]

What's the code for the form with the emailaddress field in it that initially submits to that page?

[justlukeyou]

Hi,

 

This the code which submits the information into the DB.  Im also trying to work on this but it doesn't convert the password into MD5 or send an email address.  To be honest Im not totally sure what it should do but its very slowly getting their with my skills.

 

<?php

if(isset($_POST['submit'])){
    $companyname = mysql_real_escape_string(trim($_POST['companyname']));
    $password = trim($_POST['password']);
    $password1 = trim($_POST['password1']);
    $emailaddress = mysql_real_escape_string(trim($_POST['emailaddress']));
    $error = false;
    
    if(!isset($companyname) || empty($companyname)) {
        $error = "You need to enter a username.";
    }
    $query = mysql_query("SELECT userid FROM organisermembers WHERE companyname = '".$companyname."' LIMIT 1");
    if(mysql_num_rows($query) > 0 && !$error) {
        $error = "Sorry, that username is already taken!";
    }
    if(preg_match("/[a-zA-Z0-9]{1,}$/", $companyname) == 0 && !$error) {
        $error = "The username you entered must contain only letters or numbers.";
    }
    if (strlen($companyname) > 15){    echo 'The username must not exceed 15 characters';}


    if((!isset($password) || empty($password)) && !$error) {
        $error = "You need to enter a password.";
    }
    if((!isset($password1) || empty($password1)) && !$error) {
        $error = "You need to enter your password twice.";
    }
    if($password != $password1 && !$error) {
        $error = "The passwords you entered did not match.";
    }
    
    if((!isset($emailaddress) || empty($emailaddress)) && !$error) {
        $error = "You need to enter an email.";
    }
    if(preg_match("/[a-zA-Z0-9-.+]+@[a-zA-Z0-9-]+.[a-zA-Z]+/", $emailaddress) == 0 && !$error) {
        $error = "The email you entered is not valid.";
    }
    $query = mysql_query("SELECT userid FROM organisermembers WHERE emailaddress = '".$emailaddress."' LIMIT 1");
    if(mysql_num_rows($query) > 0 && !$error) {
        $error = "Sorry, that email is already in use!";
    }
    
    if(!$error) {
        $query = mysql_query("INSERT INTO organisermembers (companyname, password, emailaddress) VALUES ('".$companyname."', '".mysql_real_escape_string(md5($password))."', '".$emailaddress."')");
        if($query) {
            $message = "Hello ".$_POST['companyname'].",\r\n\r\nThanks for registering! We hope you enjoy your stay.\r\n\r\nThanks,\r\nJohn Doe";
            $headers = "From: ".$website['name']." <".$website['emailaddress'].">\r\n";
            mail($_POST['emailaddress'], "Welcome", $message, $headers);
            setcookie("user", mysql_insert_id(), $time);
            setcookie("pass", mysql_real_escape_string(md5($password)), $time);
            header("Location: index.php");
        } else {
            $error = "There was a problem with the registration. Please try again.";
        }
    }
}

?>

[justlukeyou]

Actually sorry it is applying MD5 to 'password' but not to 'password1'.  Should apply MD5 to password 1 also?

[PFMaBiSmAd]

You need to stick to one problem at a time and solve it before moving onto the next problem.

[justlukeyou]

Well I suppose I need to enter the information correctly.

 

Do I need to even story the second password? I take it I dont?

 

 

[darkfreaks]

i am working on correcting the errors in the provided code. give me some time and i will paste the corrected code.

[PFMaBiSmAd]

Posting code with the short-comings fixed in it would do nothing toward finding out why the code is taking the execution path that gives an "Invalid email address entered." message.

 

Some possibilities -

 

1) There's no database connection and php's error_reporting/display_errors are turned off while debugging so that php isn't helping find what might be failing when the code runs.

 

2) The database table name and/or column names are not correct or the database table field length isn't long enough to hold the data.

 

3) The field in the form that submits to the password reset code isn't using the field name that the code expects, which is why someone asked for the form code that initially submits to the password reset code.

 

4) The actual email address saved in the database table is different than the one being entered and/or some white-space is being included before/after the email address being entered (the password reset code isn't trimming the entered values before using them.)

[darkfreaks]

using phplint rewrote the code not to say it is "completely" error free i think i got it down to 5 errors 1 warning. but completely better than what you had before

 

<?php
array_walk_recursive( $_POST, 'mysql_real_escape_string' );
$submit= isset($_POST['submit']) ? (string)($_POST['submit']) : '';
$user_id=intval($_GET['i']);
if($_GET['m'] === "link") {
    $query = mysql_query("SELECT * FROM organisermembers WHERE userid = '".$user_id."' LIMIT 1");
    if(mysql_num_rows($query) > 0) {
        $row = mysql_fetch_array($query);
        $r = md5($row['companyname'].$row['password']);
        if($r === $_GET['r']) {
            if($submit!==0) {
            	    $password = md5(serialize($_POST["password"]));
                if(!isset($password)) {
                    echo "Your password could not be changed because it was empty.";
                } else {
                    mysql_query("UPDATE organisermembers SET password = '".$password."' WHERE userid = '".$row['userid']."'");
                    echo "Your password has been changed. You can now <a href=\"login.php\">Login</a>.";
                }
            } else {
                echo "<form action=\"forgot.php?m=link&i=".$user_id."&r=".$_GET['r']."\" method=\"post\"><label for=\"password\">New Password:</label> <input type=\"password\" name=\"password\" /><br /><br /><input type=\"submit\" name=\"submit\" value=\"Save\" /></form>";
            }
        } else {
            echo "The link that you followed was invalid. Please try copying and pasting the link into your address bar manually.";
        }
    } else {
        echo "The link that you followed was invalid. Please try copying and pasting the link into your address bar manually.";
    }
    exit;
}


   
    $query = mysql_query("SELECT * FROM organisermembers WHERE emailaddress = '".$emailaddress."' LIMIT 1");
    if(mysql_num_rows($query) > 0) {
        $row = mysql_fetch_array($query);
        $message = "Hello ".$row['companyname'].",\r\n\r\nPlease follow this link to reset your password: ".$website['url']."/siteinfo/forgot.php?m=link&i=".$row['userid']."&r=".md5($row['companyname'].$row['password'])."\r\n\r\nThanks,\r\nJohn Doe";
        $headers = "From: ".$website['companyname']." <".$website['emailaddress'].">\r\n";
        if(mail($emailadress, "Password Reset", $message, $headers)) {
            $success = "Instructions for resetting your password have been emailed to you.";
        } else {
            $error = "There was a problem sending the email. Please try again.";
        }
    } else {
        $error = "No Email Adress Was Entered!";
    }


?>

[justlukeyou]

Hi,

 

This is the information I have. 

 

table:                  organisermembers

 

companyname:    testcompany

userid:                1

emailaddress:      test@example.com

password:          password

 

I dont understand what this part does from either piece of code.  The MD5 is applied to only the password.

 

   $r = md5($row['companyname'].$row['password']);

 

Is there a simple method of retrieving a password with an email.

[darkfreaks]

why do you need to md5 a company name? makes no sense.

 

 

try taking your time looking at the below tutorial provided perhaps you can figure out what you are doing wrong.

 

 

creating a reset password script

 

 

[PFMaBiSmAd]

It's generating a hash based on existing values in the database, rather than generating and storing a hash separately, so that when the password reset link is visited the hash from the link can be checked against the database information.

[justlukeyou]

Thanks, I shall look that through.

 

Thats what I dont understand, it should only have MD5 on the password.

 

I take it the code is supposed to retrieve information associated with the email address only with code to reset password.

 

Isn't this common place?

[justlukeyou]

It's generating a hash based on existing values in the database, rather than generating and storing a hash separately, so that when the password reset link is visited the hash from the link can be checked against the database information.

 

Whats a hash.

 

When I try to use the latest code it returns this error:

 

Warning: mysql_real_escape_string() expects parameter 2 to be resource.

 

It also says No Email Adress Was Entered!

 

 

[darkfreaks]

that is because it needs the database connection as the second parameter or a require somewhere above where your calling mysql_real_escape_string

[PFMaBiSmAd]

When I try to use the latest code it returns this error:

 

Warning: mysql_real_escape_string() expects parameter 2 to be resource.

 

That's because  the following line of code is nonsense - array_walk_recursive( $_POST, 'mysql_real_escape_string' );

 

The call-back function is passed the array value and array key as parameters one and two when it is called. Since the second parameter to the mysql_real_escape_string function is defined to be the optional connection link, it cannot be used in this way. The 'mysql_real_escape_string' function also isn't written to modify by reference the first parameter it is passed, so even if the second parameter wasn't an issue, none of the passed data values would get escaped.

[Christian F.]

justlukeyou: If you had read the article I linked you to, you would have seen that there is a complete login class available there. It will also explain all of the terms that you're wondering about, and is only more of a reason as to why you should read the article. Even if you're not interested in writing your own login system, which I agree you should not do.

[PFMaBiSmAd]

It also says No Email Adress Was Entered!

 

That's because the code that darkfreaks hacked up no longer tests if it was submitted to by the 'enter your email to reset your password' form that initially submits to that page and the code now incorrectly indicates that no email was entered when in fact that execution branch means that the entered email address (if any) wasn't found (the original error message your code had at that point was - "Invalid email address entered".)

[KevinM1]

Whats a hash.

 

 

Okay, before you go any further, you really need to sit down and go over basic web security.  Start here: http://www.amazon.com/Pro-PHP-Security-Application-Implementation/dp/1430233184/ref=sr_1_2?s=books&ie=UTF8&qid=1347297030&sr=1-2&keywords=php+security