wintallo Posted December 28, 2006 Share Posted 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]<?phpif ( 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 Link to comment https://forums.phpfreaks.com/topic/32019-change-password-script/ Share on other sites More sharing options...
bljepp69 Posted December 28, 2006 Share Posted December 28, 2006 Your submit button is named "Submit" but you check for $_POST['submit'] - capitalization matters. Submit != submit ;) Link to comment https://forums.phpfreaks.com/topic/32019-change-password-script/#findComment-148640 Share on other sites More sharing options...
Psycho Posted December 28, 2006 Share Posted 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]<?phpif ( 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] Link to comment https://forums.phpfreaks.com/topic/32019-change-password-script/#findComment-148644 Share on other sites More sharing options...
wintallo Posted December 28, 2006 Author Share Posted 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 Link to comment https://forums.phpfreaks.com/topic/32019-change-password-script/#findComment-148876 Share on other sites More sharing options...
Psycho Posted December 28, 2006 Share Posted 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. Link to comment https://forums.phpfreaks.com/topic/32019-change-password-script/#findComment-149028 Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.