Jump to content

Change Password Script


wintallo

Recommended Posts

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>&nbsp;</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
Link to comment
https://forums.phpfreaks.com/topic/32019-change-password-script/
Share on other sites

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>&nbsp;</td>
    <td><input type="submit" name="submit" value="Submit" /></td>
  </tr>
</table>
</form>
</body>
</html>[/code]
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
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.

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.