someone check my code for MD5 please

[jacko_162]

i have the following code:

 

<?php
if (isset($_POST['submit'])) {
$username = $_POST['username'];
$password = $_POST['password'];
$newpassword = $_POST['newpassword'];
$confirmnewpassword = $_POST['confirmnewpassword'];
$db_password = md5($newpassword);


$result = mysql_query("SELECT * FROM members WHERE login='$username'");
if(!$result) 
{ 
echo "The username you entered does not exist"; 
} 
else 

if($password!= mysql_result($result, 0)) 
{ 
echo "You entered an incorrect password"; 
} 
if($newpassword=$confirmnewpassword) 
    $sql=mysql_query("UPDATE members SET passwd='$db_password' where login='$username' AND member_id='$_SESSION[sESS_MEMBER_ID]'"); 
    if($sql) 
    { 
    echo "Congratulations You have successfully changed your password"; 
    }
else
{ 
echo "The new password and confirm new password fields must be the same"; 
}  }
?>
<form enctype="multipart/form-data" action="<? $_SERVER['PHP_SELF']; ?>" method="post">
<fieldset><legend>Enter your information in the form below:</legend>

<p><b>User Name:</b> <input type="text" name="username" size="10" maxlength="20" /></p>

<p><b>Current password:</b> <input type="password" name="password" size="20" maxlength="20" /></p>

<p><b>New password:</b> <input type="password" name="newpassword" size="20" maxlength="20" /></p>

<p><b>Confirm New password:</b> <input type="password" name="confirmnewpassword" size="20" maxlength="20" /></p>
</fieldset>

<div align="center"><input type="submit" name="submit" value="change My password" /></div>

</form> 

 

its so a user can change his/her password.

 

i got it to change the password and UPDATE the database with a new password with MD5 encyption, but the checks are all screwed.

 

it give me erros saying password is not correct (i assume cause its stored in the database as MD5 and doesnt check for that?!)

 

is there anyway of reading what the user entered into the "current password box" turning it into MD5 and checking against the one in database? before perfomring the sql commands?

[aeroswat]

Hey there

You must use the md5 function on the password you are checking as well.

[XeNoMoRpH1030]

Well, hopefully you are storing the password in the database with MD5, so then what you are doing should work. If not, I highly recommend doing so.

 

Sorry, just read it and it seems you are, so that's good.

 

One thing that caught my eye

if($newpassword=$confirmnewpassword) 

 

That should be == right?

[aeroswat]

Well, hopefully you are storing the password in the database with MD5, so then what you are doing should work. If not, I highly recommend doing so.

 

Sorry, just read it and it seems you are, so that's good.

 

One thing that caught my eye

if($newpassword=$confirmnewpassword) 

 

That should be == right?

 

Lol this too >< i didn't catch that

 

you are also missing the end brace on that if statement. Right before the else. Also whenever you do else's and else if's you should do them just like this

 

if(bla bla) {

  do this

} else{

  do that

}

 

if(bla bla) {

  do this

} else if(bla bla) {

  do that

}

[jacko_162]

Hey there

You must use the md5 function on the password you are checking as well.

 

hello again aero

 

i change the if($newpassword=$confirmnewpassword)  to include the additional "="

 

how do i add the MD5 check to the "current password" field when checking against the password already in the database?

 

i assume i have to edit;

 

$result = mysql_query("SELECT passwd FROM members WHERE login='$username'");
if(!$result) 
{ 
echo "The username you entered does not exist"; 
} 
else 

if($password!= mysql_result($result, 0)) 
{ 
echo "You entered an incorrect password"; 
} 

[aeroswat]

Hey there

You must use the md5 function on the password you are checking as well.

 

hello again aero

 

i change the if($newpassword=$confirmnewpassword)  to include the additional "="

 

how do i add the MD5 check to the "current password" field when checking against the password already in the database?

 

I'm guessing this is where you are checking the password

if($password!= mysql_result($result, 0)) 
{ 
echo "You entered an incorrect password"; 
}

 

Just change to this

 

if(md5($password)!= mysql_result($result, 0)) 
{ 
echo "You entered an incorrect password"; 
}

[XeNoMoRpH1030]

Not sure about the format in doing If-else statements as that is generally personal preference, but definitely fixing the coding errors should help.

 

Another thing I noticed, you just compare $_POST['password'] (set to $password) to the MD5 password from the database.

 

Edit

Beat me to it...

[jacko_162]

thats fixed it aero,

 

only problem now is if i enter "current password" and enter it wrong on purpose it still runs the query and updates the database?

[aeroswat]

thats fixed it aero,

 

only problem now is if i enter "current password" and enter it wrong on purpose it still runs the query and updates the database?

 

Change ur code to this

 

<?php
if (isset($_POST['submit'])) {
$username = $_POST['username'];
$password = $_POST['password'];
$newpassword = $_POST['newpassword'];
$confirmnewpassword = $_POST['confirmnewpassword'];
$db_password = md5($newpassword);

$result = mysql_query("SELECT * FROM members WHERE login='$username'");
if(!$result) { 
   echo "The username you entered does not exist"; 
} 
else if(md5($password)!= mysql_result($result, 0)) { 
   echo "You entered an incorrect password"; 
} else if($newpassword==$confirmnewpassword) {
   $sql=mysql_query("UPDATE members SET passwd='$db_password' where login='$username' AND member_id='$_SESSION[sESS_MEMBER_ID]'"); 
    if($sql) { 
       echo "Congratulations You have successfully changed your password"; 
    }
} else { 
echo "The new password and confirm new password fields must be the same"; 
}  
}
?>

[jacko_162]

thats fixed it aero,

 

only problem now is if i enter "current password" and enter it wrong on purpose it still runs the query and updates the database?

 

Change ur code to this

 

<?php
if (isset($_POST['submit'])) {
$username = $_POST['username'];
$password = $_POST['password'];
$newpassword = $_POST['newpassword'];
$confirmnewpassword = $_POST['confirmnewpassword'];
$db_password = md5($newpassword);

$result = mysql_query("SELECT * FROM members WHERE login='$username'");
if(!$result) { 
   echo "The username you entered does not exist"; 
} 
else if(md5($password)!= mysql_result($result, 0)) { 
   echo "You entered an incorrect password"; 
} else if($newpassword==$confirmnewpassword) {
   $sql=mysql_query("UPDATE members SET passwd='$db_password' where login='$username' AND member_id='$_SESSION[sESS_MEMBER_ID]'"); 
    if($sql) { 
       echo "Congratulations You have successfully changed your password"; 
    }
} else { 
echo "The new password and confirm new password fields must be the same"; 
}  
}
?>

 

worked a treat, another problem solved

 

now i gotta read it see what you changed else i wont learn anything

[aeroswat]

You needed a chain of else if's. Notice the $newpassword==$confirmpassword part is now an else if. Otherwise if it was just an if outside of the previous if's then it would be checked independent of what the other if's produced. This way if the two previous if's were good to go (false) then and only then you get to check the new password and insert it into the database.

[jacko_162]

aaah now i get it

 

if i enter a fake username it gives me the error for incorrect password, shouldnt the

if(!$result) {

code be saying if $result = 0 or something?

[aeroswat]

aaah now i get it

 

if i enter a fake username it gives me the error for incorrect password, shouldnt the

if(!$result) {

code be saying if $result = 0 or something?

 

Should be

if(!mysql_num_rows($result))