Changing password and md5 issues

I am building a simple discussion forum and having problems with a feature that allows the user to get a new password if they forgot their old one.

They are not logged in, but click on a link “forgot password?” that takes them to a new page where they enter their username and hit a submit button. On submit, a random 6 character password is generated, emailed to them and updated in the database. My problem is that even though this seemed to work, the user couldn’t log in with their new password. I checked and when I used MD5 manually on the random password sent by email, I get a different hashed password than the one entered in the database. What is happening? The code for this is below:

?php
require_once("inc/session.php");
require_once("inc/connect_db.php");
require_once("inc/functions.php");

include_once("inc/header.php");
?>

<h2>Password Reset</h2>
<p>Please enter your first and last name.</p>
	<form method="post" action="new_password.php">
		<table>
			<tr>
				<td class="label">Username:</td>
				<td><input type="text" name="username" id="username" class="textbox" value="<?php print $_POST['username']; ?>"/><td>
			</tr>
			<tr>
				<td>&nbsp;</td>
				<td><input type="submit" name="submit" id="submit" class="button" value="Reset Password"/><td>
			</tr>
		</table>
	</form>
	
<?php	
	if ($_POST['submit']) {
	
		// record name of user
		$username = trim(mysql_prep($_POST['username']));
		$name_parts = explode(" ", $username);
		$first_name = $name_parts[0];
		$last_name = $name_parts[1];
		
		// get the user's email from the database ... also use the results of the query to check whether the user is legitimate
		$query = "SELECT email1
		FROM users
		WHERE first_name = '$first_name' 
		AND last_name = '$last_name'
		LIMIT 1 ";
	
		$result = mysql_query($query, $mysql_link);
		
		// if the user's name is in the database, create the new password and mail it to him
		if (mysql_num_rows($result) >0) { 
	
			$row = mysql_fetch_row($result);
			$email = stripslashes($row[0]);
		
			// create temporary password consisting of 6 random lower and upper case letters
			function randLetter() 	{
				$int = rand(0,51);
				$a_z = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
				$rand_letter = $a_z[$int];
				return $rand_letter;
			}
		
			$temp_password = ' ';

			for ($x=1; $x<=6; $x++) {
				$temp_password .= randLetter();
			}
			
			print("<p>Your new temporary password has been sent to you, $first_name. Please check your email shortly, and follow the instructions that are included in the message.</p>");
	
			// send email	
			$to = $email;
			$subject = "Password Reset for ****** Forum";
			$message = "<p>Hi $first_name,</p>
			<p>You have been given a temporary password:&nbsp;&nbsp;&nbsp;$temp_password</p>
			<p>Use this password to log in to the <a href=\\"http://*******.com/******\\">******' Forum</a>,
			and change your password right away to a password that is easier for you to remember.</p>
			<p>Thank you for your interest in the ******' Forum</p>";

			$headers = "From: ***@********.org\\r\
";
			$headers .= "Content-type: text/html\\r\
";

			mail($to, $subject, $message, $headers);
	
			// update the database with the temporary password
			$password = md5($temp_password);
			//$password = $temp_password;
	
			$query = "UPDATE users
			SET password = '$password'
			WHERE first_name = '$first_name' 
			AND last_name = '$last_name' ";
	
			$result = mysql_query($query, $mysql_link);
		} else {
		// the username could not be found in the database.
		print("<p>$first_name, we were unable to locate your name in our records. If you think you typed your name incorrectly, make sure your caps lock is off and try again.
		Otherwise, check with the site administrator to find out why your name is not included in the records.</p>");
		
		}
	}
?>

<?php
include_once("inc/footer.php");
?>

Check the password; change it, then check to make sure it has actually changed.

What’s your login code look like?

PS: Dont check for $_POST[‘submit’]. Subject to the IE Submit Bug.

I did check … I recorded the original hashed password, ran the script, and recorded the new password after the database was updated, and what I got when I applied md5 to the new password manually. All three were different.

Here’s my index page where the user logs in:

<?php
require_once("inc/session.php");
require_once("inc/connect_db.php");
require_once("inc/functions.php");
// after login is successful, three session variables are created:
// $_SESSION['username'] is the username of the person logged in
// $_SESSION['user_id'] is the id of the person logged in
// $_SESSION['role'] is either admin, chair, user or inactive
?>

<?php

if (logged_in()) {
	redirect_to("main.php");
}

include_once("inc/form_functions.php");

// start form processing
if (isset($_POST['submit'])) {
	$errors = array();
	
	//perform validations on the form data
	$required_fields = array('username', 'password');
	$errors = array_merge($errors, check_required_fields($required_fields, $_POST));

	$fields_with_lengths = array('username' => 30, 'password' => 20);
	$errors = array_merge($errors, check_max_field_lengths($fields_with_lengths, $_POST));

	$username = trim(mysql_prep($_POST['username']));
	$name_parts = explode(" ", $username);
	$first_name = $name_parts[0];
	$last_name = $name_parts[1];
	$password = md5(trim(mysql_prep($_POST['password'])));
	
	if (empty($errors)) {
		// check database to see if username and the password exist there.
		$query = "SELECT id, first_name, last_name, password, role
		FROM users
		WHERE role <> 'inactive'
		AND first_name = '$first_name'
		AND last_name = '$last_name'
		AND password = '$password'
		LIMIT 1 ";
		$result = mysql_query($query, $mysql_link);
		
		confirm_query($result);
		
		if (mysql_num_rows($result) == 1) {
			// username/password authenticated
			$found_user = mysql_fetch_array($result);
			$_SESSION['user_id'] = $found_user['id'];
			$_SESSION['username'] = $found_user['first_name'] . " " . $found_user['last_name'];	
			$_SESSION['role'] = $found_user['role'];
			redirect_to("main.php");
		} else if ($_SESSION['role'] == 'inactive') { 
			// The person is no longer an active member of the deacons board
			$msg = "<p class=\\"message\\">You are no longer an active member of the deacons board, and do not
			have access to this forum. Please contact the chairperson of the board if you have any questions.</p>";
		} else { 
			// username/password combo was not found in the database
			$msg = "<p class=\\"message\\">Username/password combination is incorrect.<br />
			Please make sure your caps lock key is off and try again.</p>";
		}
	} else {
		if (count($errors) == 1) {
			$msg = "There was 1 error in the form.";
		} else {
			$msg = "There were " . count($errors) . " errors in the form.";
		}
	}
	
} else { // Form has not been submitted

	if (isset($_GET['logout']) && $_GET['logout'] == 1) {
		$msg = "You are now logged out.";
	}
	$username = " ";
	$password = " ";	
}	
?>

<?php
include_once("inc/header.php");
?>
<div id="toDo" style="width: 410px; float:right; border: 1px solid #ddd7dd; padding: 10px; margin-top: 30px;">
	<h4>For Testing:</h4>
		<p>Use the following member information for testing purposes:</p>
		<p>Member: Sam White &nbsp;&nbsp;&nbsp;password: samwhite</p>
		<p>Forum Admin: John Doe &nbsp;&nbsp;&nbsp;password: johndoe</p>
		<p>Inactive Member: Nan Tucket &nbsp;&nbsp;&nbsp;password: nantucket</p>
		<p style="margin-top: 20px;"><a href="notes.php">Development Notes >></a></p>
		
</div>

<h2>Member Login</h2>
<?php
	if (!empty($msg)) {
		print("<p class=\\"message\\">" . $msg . "</p>");
	}

?>
<?php
	if (!empty($errors)) { 
		display_errors($errors); 
	}
?>
<p>Please enter your first and last name, and password to log in:</p>
<form name="login" action="index.php" method="post" id="loginForm">
	<table>
		<tr>
			<td class="label">Username:</td>
			<td><input type="text" name="username" id="username" class="textbox" /><td>
		</tr>
		<tr>
			<td class="label">Password:</td>
			<td><input type="password" name="password" id="password" class="textbox" /></td>
		</tr>
	</table>	
	<p id="newpassword"><a href="new_password.php">Forgot your password?</a></p>	
	<input type="submit" name="submit" id="loginSubmit" class="button" value="Login" />
</form>
<div class="clear"></div>

<?php
include_once("inc/footer.php");
?>

and the code I use to prepare my variables:

function mysql_prep( $value ) {
		$magic_quotes_active = get_magic_quotes_gpc();
		$new_enough_php = function_exists( "mysql_real_escape_string" ); // i.e. PHP >= v4.3.0
		if( $new_enough_php ) { // PHP v4.3.0 or higher
			// undo any magic quote effects so mysql_real_escape_string can do the work
			if( $magic_quotes_active ) { $value = stripslashes( $value ); }
			$value = mysql_real_escape_string( $value );
		} else { // before PHP v4.3.0
			// if magic quotes aren't already on then add slashes manually
			if( !$magic_quotes_active ) { $value = addslashes( $value ); }
			// if magic quotes are active, then the slashes already exist
		}
		return $value;
	}

Only thing that strikes me is that you’re not running trim and mysql_prep (whatever that does) on the new password…

I’ll try that. I didn’t think it mattered, because the random password has no spaces, or special characters that need escaping.

Later … I used trim and mysql_prep (I posted the code for that function too) on the randomly generated password and that did the trick. Thank you very much!!

Now it works, but it is really bugging me because I don’t really understand why it would make a difference, because of how my random password was constructed. One last loose end to take care of.

Could it have something to do with the space you’re adding to the beginning of the temp_password?

Sorry for being a little off-topic but wouldn’t it be better to provide users a way of setting a new password instead of generating some random garbage they won’t be able to remember? Sending a password via email is not very secure as it is sent to the email provider’s server and then to the user’s computer, or sometimes several computers if they check their mail in multiple places - and encryption cannot be guaranteed.

I think a more sensible approach is to generate a one-time unique link that you send to the user and they will be able to use it once to set a new password on your site. As the link expires pretty quickly it poses no security risk even if someone else reads the user’s inbox.

Just some food for though!

A lot of sites do this. It’s a way of making sure the customer is who they say they are by interacting with the email address that they registered with. So long as they are told to go back to their user account interface on the site and reset the password to something they can remember they will be fine. I am aware of the dangers of plain text passwords in e-mail but it is intended to be temporary and as long as the customer knows that that should be all that is necessary.