Forgot Password: Is my code okay? Newbie

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
tristan5522
Forum Newbie
Posts: 1
Joined: Thu Apr 24, 2014 9:25 pm

Forgot Password: Is my code okay? Newbie

Post by tristan5522 »

Hey everyone, this is my first post. Currently this code resets the user's password and replaces it with some random code in the database. Not sure what I am doing wrong, any help would be greatly appreciated.

Code: Select all

<?php
$heading = "Forgot Password";

if(isset($_GET['action']) && $_GET['action'] == "fpwd")
{
if(count($_POST) > 0)
{
if(isset($_POST['user_email']))
{
$email_address = $_POST['user_email'];

$sqlemail = "select user_email from ".TABLE_user." where user_email = '$email_address'";
$resemail = mysql_query($sqlemail);

$password = "user".rand(1000,50000);

$sql_update = "update ".TABLE_user." set 'password' = '".md5($password)."' where 'user_email' = '$email_address'";
$res = mysql_query($sql);


$to = $email_address;
$subject = 'Reset Password';
$message = 'Your new password: '.$password;
$headers = 'From: '.STORE_EMAIL.'' . "\r\n";

if(mail($to, $subject, $message, $headers))
{
fw_goto_page_header(fw_create_link(FILENAME_FORGOT_PWD,'msg=1'));
}

}
}
}

?>
User avatar
Celauran
Moderator
Posts: 6427
Joined: Tue Nov 09, 2010 2:39 pm
Location: Montreal, Canada

Re: Forgot Password: Is my code okay? Newbie

Post by Celauran »

Code: Select all

$sqlemail = "select user_email from ".TABLE_user." where user_email = '$email_address'";
You're selecting a column whose value you already know? Odd. More importantly, you're passing user data directly into a query. This will end badly. Sanitize your inputs. Always.

The mysql_ functions have been deprecated and should not be used. Consider using PDO instead, ideally in conjunction with prepared statements.

md5 is entirely unsuitable for hashing passwords. Use bcrypt.

Lastly, you shouldn't be emailing users their passwords. A better solution would be to email them a link where they can choose a new password.
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Forgot Password: Is my code okay? Newbie

Post by social_experiment »

Celauran wrote:Lastly, you shouldn't be emailing users their passwords.
Yeah, this is a bad practise yet it is so popular. I recently worked on a e-commerce system and even the payment gateway company emailed me a password.

Code: Select all

<?php $password = "user".rand(1000,50000); ?>
This isn't very effective; rather go for something different; your reset password should contain alpha-numeric characters and be at least 10 characters in length.

Code: Select all

<?php
function createNewPassword()
{
		$randomData = array(    // Array of characters used to create password		
		'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 
		'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 
		'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '1',
		'2', '3', '4', '5', '6', '7', '8', '9', '0', 
		'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 
		'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 
		's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '!',
		'@', '#', '$', '%', '^', '&', '*', '(', ')',
		'-', '=', '`','\'', '"', '{', '}', '[', ']', 
		'<', '>', '/', '?', '.', '|', '_', 
		);

	$randomString = false;			
			
	shuffle($randomData);
	$array = array_rand($randomData, 15);
			
	foreach ($array as $key => $value) 
	{
		$randomString .= $randomData[$key];
	}
	
	return $randomString;
}
?>
As a rule you should as the user to reset their password once you have sent a reset password email; before they are even allowed to login, that ensures they change the password immediately.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
User avatar
Celauran
Moderator
Posts: 6427
Joined: Tue Nov 09, 2010 2:39 pm
Location: Montreal, Canada

Re: Forgot Password: Is my code okay? Newbie

Post by Celauran »

I don't know that you want to be generating a new password at all. You can't mail it to them, so what's it for? You don't necessarily want to overwrite the original password; what if they end up remembering it?
Post Reply