Change Password Script

wintallo · December 28, 2006

Hi, today I wrote a PHP script that would allow the users to change their password. When I tested it out, all that happened was the screen flashed like it was reloading and nothing changed. When I tried to log in using my login script, the old password was still active. Here's the code.

[code]
<?php

if ( isset($_POST['submit']) ) {

if ( $_POST['oldpassword'] != "" || $_POST['newpassword1'] != "" || $_POST['newpassword2'] != "" ) {

/////////////////////////////////

$dbhost = 'localhost';
$dbuser = '*******';
$dbpass = '*******';
$dbname = '*******';

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Error connecting to mysql');
mysql_select_db($dbname);

/////////////////////////////////

$user = "testuser"; // I'll add a function to this so that 4user is equal to the current user.

$query = "SELECT password FROM users WHERE login = '$user'";

$result = mysql_query($query);

$row = mysql_fetch_array($result);

$encuserpassword = $row['password'];

$encuserinput = sha1($_POST['oldpassword']);

mysql_close($conn);

if ( $encuserinput == $encuserpassword ) {

if ( $_POST['newpassword1'] == $_POST['newpassword2'] ) {

/////////////////////////////////

$dbhost = 'localhost';
$dbuser = '*******';
$dbpass = '*******';
$dbname = '*******';

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Error connecting to mysql');
mysql_select_db($dbname);

/////////////////////////////////

$user = "testuser"; // I'll add a function to this so that 4user is equal to the current user.

/////////////////////////////////

$encnewpassword = sha1($_POST['newpassword1']);

$query = "UPDATE users SET password = '$encnewpassword' WHERE login = '$user'";

$result = mysql_query($query);

echo "Your password has been changed.";

mysql_close($conn);

} else {

$errormessage = "The two new passwords don't match.";

}

} else {

$errormessage = "Your old password isn't valid.";

}

} else {

$errormessage = "Please fill in all fields.";

}
}

?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<title>Change Password</title>
</head>
<body>
<br />
<?php echo $errormessage; ?>
<form name="password_change" method="post" action="">
<table width="350" border="0">
<tr>
<td>Enter Old Password: </td>
<td><input name="oldpassword" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td width="220">Enter New Password: </td>
<td width="120"><input name="newpassword1" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td>Re-Enter New Password: </td>
<td><input name="newpassword2" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td> </td>
<td><input type="submit" name="Submit" value="Submit" /></td>
</tr>
</table>
</form>
</body>
</html>
[/code]
Thanks for reading! Hope you guys can help!

-Joel

bljepp69 · December 28, 2006

Your submit button is named "Submit" but you check for $_POST['submit'] - capitalization matters. Submit != submit ;)

Psycho · December 28, 2006

There's several problems.

1) [color=blue]if ( isset($_POST['submit']) ) {[/color] the "submit" has a lower case "s", but in your form you use [color=blue]name="Submit"[/color] with an upper case "s"

2) [color=blue]$_POST['oldpassword'] != "" || $_POST['newpassword1'] != "" || $_POST['newpassword2'] != ""[/color] You shoud be using "&&" between each conditional. Right now this will pass any one or more of the fields has a value. You want to ensure that ALL the fields have a value.

3) You should move the check to see if the two new passwords match right after the conditional in item two above - no reason to hit the database if they didn't fill in the form correctly.

Here is how I would do it. I would also add a lot more error handling - this also assumes the "login" column is a uniqu field. Otherwise you should pull the user's record by login and password.
[code]<?php

if ( isset($_POST['submit']) ) {
if ( !$_POST['oldpassword'] || !$_POST['newpassword1'] || !$_POST['newpassword2'] ) {
$errormessage = "You must complete all required fields.";
} else {
if ( $_POST['newpassword1'] != $_POST['newpassword2'] ) {
$errormessage = "The two new passwords don't match.";
} else {
$user = "testuser"; // I'll add a function to this so that $user is equal to the current user.

$dbhost = 'localhost';
$dbuser = '*******';
$dbpass = '*******';
$dbname = '*******';

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ("Error connecting to mysql".mysql_error());
mysql_select_db($dbname) or die ("Error selecting database: ".mysql_error());

$query = "SELECT password FROM users WHERE login = '$user'";
$result = mysql_query($query) or die (mysql_error());
$row = mysql_fetch_array($result);
$encuserpassword = $row['password'];

$encuserinput = sha1($_POST['oldpassword']);

if ( $encuserinput != $encuserpassword ) {
$errormessage = "You have entered the wrong password.";
} else {
$encnewpassword = sha1($_POST['newpassword1']);
$query = "UPDATE users SET password = '$encnewpassword' WHERE login = '$user' AND password = '$encuserpassword'";
$result = mysql_query($query);
echo "Your password has been changed.";
}
mysql_close($conn);
}
}
}

?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<title>Change Password</title>
</head>
<body>
<br />
<?php echo $errormessage; ?>
<form name="password_change" method="post" action="">
<table width="350" border="0">
<tr>
<td>Enter Old Password: </td>
<td><input name="oldpassword" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td width="220">Enter New Password: </td>
<td width="120"><input name="newpassword1" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td>Re-Enter New Password: </td>
<td><input name="newpassword2" type="password" size="20" maxlength="15" /></td>
</tr>
<tr>
<td> </td>
<td><input type="submit" name="submit" value="Submit" /></td>
</tr>
</table>
</form>
</body>
</html>[/code]

wintallo · December 28, 2006

Thanks a ton for the help!

[quote author=mjdamato link=topic=120099.msg492451#msg492451 date=1167286794]
this also assumes the "login" column is a uniqu field. Otherwise you should pull the user's record by login and password.
[/quote]

Yes, my script that allows the users to register checks if the username is already used. So all usernames are unique. I have one question: Is there an security problems with this script, for example, can a hacker use this form to do any malicious things to my site? I'm asking this because I just finished re-building another one of my sites after a hacking scenario. I'm just want to be really cautious about my scripts. Thanks!

-Joel

Psycho · December 28, 2006

You will need to ensure that the $user value - however you come up with it - can't be hacked.

You should always use mysql_real_escape_string() when using any user input in a query to prevent SQL injection. However, you are hashing the values before you use them in a query, so it shouldn't be a problem.

Sign In

Change Password Script

Recommended Posts

wintallo

Link to comment

Share on other sites

bljepp69

Link to comment

Share on other sites

Psycho

Link to comment

Share on other sites

wintallo

Link to comment

Share on other sites

Psycho

Link to comment

Share on other sites

Join the conversation

Browse

Activity

Important Information