Help with a change password form md5 security

[cjohnson9]

Hello freaks,

 

I have been following and using phpfreaks.com for some time now, but this will be my first post. I am trying to add a way to change passwords in my  mysql database and am running into trouble.

 

Basically I would like users to enter their username, current password, new password, and confirmed new password. Then I would like php to validate that the current password matches the database, and also that the new password and confirmed passwords match.

 

 

 

I have written some code and will paste my php work below:

 

<?php
session_start();
?>
  <?php 
  
  if ($_SESSION['username'])
  {
?>
  
<form name="form" method="post" action="account.php"> 
  <p><h4>Change Password</h4>
<table border="0">
<tr><td>Username:</td><td>
   <input type="text" name="username" maxlength="20"></td></tr>
<tr><td>Old password:</td><td>
   <input type="password" name="oldpassword" maxlength="20"></td></tr>
   <tr><td>New password:</td><td>
   <input type="password" name="pass1" maxlength="20"></td></tr>
   <tr><td>Confirm password:</td><td>
   <input type="password" name="pass2" maxlength="20"></td></tr>   
</table>
<input type="submit" name="change"  class="buttonChange" value="Change" />
</form>

<?php } ?>

  <?php
$submit = $_POST['change'];

$username = strtolower(strip_tags($_POST['username']));
$pass = strip_tags($_POST['pass']);
$pass2 = strip_tags($_POST['pass2']);


if ($submit)
{

mysql_connect("localhost", "root", "c3rb3ru5") or die("Can't Connect"); 
mysql_select_db("gpj_asset") or die("Can't Connect"); 


		$pass = md5($_POST['pass']);
		$pass2 = md5($_POST['pass2']);

		if ($pass == $pass2)
		{		
			mysql_query("UPDATE user SET password = '$pass' WHERE username = '$username'");


			echo "*Success!";
		}
		else echo "*Passwords do not match!";

}

?>

 

Any direction would be greatly appreciated...

 

Thanks,

 

Clint

[Pikachu2000]

Your logic should be to first run a SELECT query against the username and old password to get the record's primary key, and verify that exactly 1 record matches. Then if the 2 fields containing the new password match, run an UPDATE query to insert it into the table. You also need to validate and sanitize the form data to add some protection against SQL injection.

[HyPeRcOmPs]

Added some tweaks and additions to it, you had hardly any sql injection security. As you can see in the script I added some built in PHP functions to accomplish this.

 

• mysql_real_escape_string() - renames slashes as so \x00, \n, \r, \, ', " and \x1a
  preg_replace() - Replaces set characters, little bit complicated to explain. http://us3.php.net/manual/en/function.preg-replace.php
  
• 
  

 

Scripts logic is as so:

 

If the username is set continue, if NOT stop the script and echo out "NOT LOGGED IN".

If the variable $submit equals "Change" continue, sql injection security measures explained above.

Look into database and find how many usernames and passwords match.

If more then zero, update database with new password specified. If NOT echo error message "Passwords do NOT match!".

 

 

Didn't test the below script but should work, let me know how it goes..

------------------------------------------------------

 

<?php
session_start();
?>
  <?php 
  $userName = $_SESSION['username'];
  $username = strtolower(strip_tags($_POST['username']));
  
  if (isset($_SESSION['username']))
  {
  	$submit = $_POST['change'];
  
  if ($submit=='Change')
{
  
    //get post
$submit = $_POST['change'];

//added injection security

//username
$username1 = mysql_real_escape_string($username);
$username2 = preg_replace("/[^A-Za-z0-9-]/","", $username1);

$username = $username2;

//pass
$pass1 = strip_tags($_POST['pass1']);
$pass33 = mysql_real_escape_string($pass1);
//can only use A-Z,a-z,0-9 no spaces
$pass44 = preg_replace("/[^A-Za-z0-9-]/","", $pass33);
//return to correct var
$pass = $pass1;

//pass2
$pass2 = strip_tags($_POST['pass2']);
$pass3 = mysql_real_escape_string($pass2);
//can only use A-Z,a-z,0-9 no spaces
$pass4 = preg_replace("/[^A-Za-z0-9-]/","", $pass3);
//return to correct var
$pass2 = $pass4;


$oldpass = $_POST['oldpassword'];

//old pass
$oldpass2 = mysql_real_escape_string($oldpass);
//can only use A-Z,a-z,0-9 no spaces
$oldpass3 = preg_replace("/[^A-Za-z0-9-]/","", $oldpass2);

mysql_connect("localhost", "root", "c3rb3ru5") or die(mysql_error()); 
mysql_select_db("gpj_asset") or die(mysql_error()); 
$runThis = "SELECT * FROM user WHERE username = '$username' && password = '$oldpass3'";
//find the amount that match
$count = mysql_num_rows($runThis);


		$pass = md5($_POST['pass']);
		$pass2 = md5($_POST['pass2']);

		if ($pass == $pass2 && $count<'0')
		{		
			mysql_query("UPDATE user SET password = '$pass' WHERE username = '$username'");


			echo "*Success!";
		}
		else echo "*Passwords do not match!";

}
?>
  
<form name="form" method="post"> 
  <p><h4>Change Password</h4>
<table border="0">
<tr><td>Username:<?php echo $userName; ?></td><td>
   <input type="text" name="username" maxlength="20"></td></tr>
<tr><td>Old password:</td><td>
   <input type="password" name="oldpassword" maxlength="20"></td></tr>
   <tr><td>New password:</td><td>
   <input type="password" name="pass1" maxlength="20"></td></tr>
   <tr><td>Confirm password:</td><td>
   <input type="password" name="pass2" maxlength="20"></td></tr>   
</table>
<input type="submit" name="change"  class="buttonChange" value="Change" />
</form>

<?php }  else {

die('NOT LOGGED IN.');
}
?>

[cjohnson9]

WOW!!!

 

thank you guys so much for the quick replies.

 

I will try this later today and will let you guys know.

 

Thanks Again!!!

[Psycho]

Your logic should be to first run a SELECT query against the username and old password to get the record's primary key, and verify that exactly 1 record matches. Then if the 2 fields containing the new password match, run an UPDATE query to insert it into the table. You also need to validate and sanitize the form data to add some protection against SQL injection.

 

That was my initial thought as well. But, when I did a little more analysis I think you could so this with just ONE update query (no SELECT query) using the username/current PW in the WHERE clause. Then just do a check of affected rows. If the result is 0, then the username and/or current password are incorrect. If the result is 1, then username/current password were correct and the value was updated. It is only reducing the queries from 2 to 1, but I try to eliminate as much overhead on the db as possible.

 

Sample code:

//process the input into variables
$username = strtolower(strip_tags(trim($_POST['username'])));
$oldpass  = strip_tags(trim($_POST['oldpass']));
$newpass1 = strip_tags(trim($_POST['newpass1']));
$newpass2 = strip_tags(trim($_POST['newpass2']));

//Validate input
$error = false;
if(empty($username) || empty($oldpass) || empty($newpass1) || empty($newpass2))
{
    $error = "All fields are required.";
}
elseif($newpass1 != $newpass2)
{
    $error = "New passwords do not match.";
}
else
{
    //No input errors, run query
    $username = mysql_real_escape_string($username);
    $oldpass  = md5($oldpass);
    $newpass1 = md5($newpass1);
    $query = "UPDATE user
              SET password = '$newpass1'
              WHERE username = '$username'
                AND password = '$oldpass'";
    $result = mysql_query($query) or die(mysql_error());
    if(mysql_affected_rows()!==1)
    {
        $error = "Username and/or password are not correct".
    }
}

//Check if any errors occured
if($error !== false)
{
    echo "<span style=\"color:red\">The following error occured: {$error}</pan>\n";
}
else
{
    echo "Password updated successfully".
}

[Pikachu2000]

That's true as long as the table has been set up properly and the inserts have been done properly from the beginning. I just prefer to check that the result is precisely what it should be, such as exactly 1 matching record exists, before doing anything else. It does require an additional query, however.

[Psycho]

I just prefer to check that the result is precisely what it should be, such as exactly 1 matching record exists, before doing anything else.

 

I agree with that, I always like to be very explicit. In this case, you have three possible outcomes: A) no records were updated which means username/password do not match, B) Only one record matched and was updated (success scenario), or C) There were multiple records that matched the username and password. If you were to get option C there are some serious problems that need to be resolved independent of the pw update script.

[Pikachu2000]

Yeah, we're on the same page here. Especially with your last sentence

[cjohnson9]

Sorry guys,

 

This makes sense but I tried both with no luck. The first block of code always tells me the passwords don't match, and the second one has a syntax error I cannot find.

 

Thanks again

[Psycho]

This makes sense but I tried both with no luck. The first block of code always tells me the passwords don't match, and the second one has a syntax error I cannot find.

 

That is not very helpful - at least post the errors you are getting. Although I do see in the code I provided I ended two lines with a period instead of a semi-colon. But, if you can't find those errors by yourself . . .

[cjohnson9]

Sorry,

 

I fixed those, but I still have an error somewhere, I don't get an error message but it always says the The following error occurred: All fields are required. I am not sure what to do sorry for the lack of information.

 

 

[cjohnson9]

I think I found the error in the first one. I think the less than sign in the if statement to check if the passwords match and there is a record. it says <0 I changed it to >0 and will test

 

Thanks again

[Psycho]

If you are getting the "all fields required" response, then one of the four variables are empty. Make sure you have form fields that correspond to the fields referenced at the beginning of the script. I didn't use all of your original form field names because I just use whatever makes sense to me when I am coding on the fly.

 

Either change the form field names to match the POST variables below OR change the POST variables below to match your form field names.

//process the input into variables
$username = strtolower(strip_tags(trim($_POST['username'])));
$oldpass  = strip_tags(trim($_POST['oldpass']));
$newpass1 = strip_tags(trim($_POST['newpass1']));
$newpass2 = strip_tags(trim($_POST['newpass2']));

[cjohnson9]

NooB error!

 

Im sorry I wasn't paying attention I used the second solution and found I had named the variable for username to email and forgot to change it back.

 

Thank You so Much Sorry I made this so difficult. This is definitely solved!

[Pikachu2000]

There's a "Topic Solved" button below that you can click . . .