What wrong with my php change password???

Question

<? 
// Inialize session
session_start();

// Check, if username session is NOT set then this page will jump to login page

if (!isset($_SESSION['username'])) 
{
header('Location: AdminLogin.php');
}
?>

<html lang="en-GB" xmlns="http://www.w3.org/1999/xhtml">
<head>
<link rel="stylesheet" href="AdminLogin.css" type="text/css" />
<title>Welcome to ASM Services Inc.</title>

<script type="text/javascript" language=JavaScript>
var message="";
function clickIE() 
{
    if (document.all) 
    {(message);return false;}}
function clickNS(e) {if 
(document.layers||(document.getElementById&&!document.all)) {
if (e.which==2||e.which==3) {(message);return false;}}}
if (document.layers) 
{document.captureEvents(Event.MOUSEDOWN);document.onmousedown=clickNS;}
else{document.onmouseup=clickNS;document.oncontextmenu=clickIE;}

document.oncontextmenu=new Function("return false")
</script>
</head>
<body>

<div class="login">
<?php
require("adminconfig.inc");
$user = $_SESSION['username'];
echo "<form name=form1 method=post>
<table width=100 border=0 align=center>
<tr>
<font size=5 face=Arial color=yellow>Change Password</font>
</tr>
<table>
    <tr>
        <td><font size=4 face=Tahoma color=yellow>Username:</font></td>
        <td><input type=text name='username1' value='$user' size=20 AUTOCOMPLETE = off ></td>
    </tr>
    <tr>
        <td><font size=4 face=Tahoma color=yellow>Password:</font></td>
        <td><input type=password name=password size=20 AUTOCOMPLETE = off></td>
    </tr>
    <tr>
        <td><font size=4 face=Tahoma color=yellow>New Password</font></td>
        <td><input type=password name=new_pass size=20 AUTOCOMPLETE = off></td>
    </tr>
    <tr>
        <td><font size=4 face=Tahoma color=yellow>Confirm Password:</font>:</td>
        <td><input type=password name=con_pass size=20 AUTOCOMPLETE = off></td>
    </tr>
</table>
<table>
    <tr>
        &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
        <input type=submit value=Ok name='btnCheck'>
        &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
        <input type=submit value=Cancel name=btnCancel onClick='this.form.reset()'>
    </tr>
</table>
</table>
</form>";
?>

<?php
require("adminconfig.inc");
$user = $_POST['username1'];
$pass = $_POST['password'];
$new_pass = trim($_POST['new_pass']);
$con_pass = trim($_POST['con_pass']);
if(isset($_POST['btnCheck']))
{
// Retrieve username and password from database according to user's input
$login = mysql_query("SELECT Log_User, Log_Pass, User_Type FROM LOG_IN WHERE  
(Log_User = '" . mysql_real_escape_string($_POST['username1']) . "') 
and 
(Log_Pass = '" . mysql_real_escape_string($_POST['password']) . "') 
and 
(User_Type = 'member')") 
or die('Query failed: ' . mysql_error() . "<br />\n$sql"); ;


//Check username and password match
if (mysql_num_rows($login) == 1) 
{
    if(trim('$new_pass') == trim('$con_pass'))
    {
        $sql=mysql_query("UPDATE log_in SET Log_Pass='$new_pass' where username='$user'"); 
        if(!$sql) 
        { 
            echo "fail updating!";
        }
        else
        {
            echo "success!";
            echo "<script type = text/javascript>";
            echo "alert('The new password has been changed successfully.');";
            echo "</script>";
        } 
    }
    else
    {
        echo "fail!";
        echo "<script type = text/javascript>";
        echo "alert('Error. New Password and Confirm Password are not the same. Please make it sure that they are the same.');";
        echo "</script>";
    }
}
}
?>

</div>

<div class="copyright">
&copy; Copyright 2011 <strong>ASM Services Inc.</strong>
</div>
</body>
</html>

This is my whole code for changing the password of the user. I really don't what's the exact error of my code. Whenever I change the password, it always brings to the "Error. New Password and Confirm Password are not the same".

You're storing the password in **plaintext**; that's the single *most* wrong thing in your code. See: http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords — Piskvor left the building, Nov 10 '11 at 06:48
I won't say it's *most* wrong thing. Personally I don't mind if my password were stored in plain text. To register on some lame site I'd use mailinator and care not of it's security — Your Common Sense, Nov 10 '11 at 06:50
How did the word "help" make it into the title? Isn't there a filter for that? — BoltClock, Nov 10 '11 at 06:54
@Col. Shrapnel: Well, *you* may be using a different, reasonably secure password for every site; alas, that is not a very typical case. With password reuse, I fear that Joe R. User will enter the same password for a lame site as for his e-mail account and his banking account. Note that the user's e-mail address is usually stored nearby in most databases. — Piskvor left the building, Nov 10 '11 at 07:01
@Piskvor as long as you don't care for the password strength, all this hashing mess is just a baby talk. It is interesting phenomenon of this blessed site - among numerous security issues, only hashing were chosen to peddle with. — Your Common Sense, Nov 10 '11 at 07:03
@Col. Shrapnel: Good point - if the password is `12345`, then the security of the backend becomes somewhat moot. (I didn't say that's the *only* problem of the code :)) — Piskvor left the building, Nov 10 '11 at 07:12

codaddict · Accepted Answer · 2011-11-10T06:49:19.773

3

Your current code has:

if(trim('$new_pass') == trim('$con_pass')) {
  // passwords match
} else {
  // passwords don't match
}

You are comparing the strings '$new_pass' & '$con_pass' and not the variables $new_pass & $con_pass. Also don't use should not use trim as the user might have space in his passwords.

Change

if(trim('$new_pass') == trim('$con_pass'))

to

if($new_pass == $con_pass)

Also you read the passwords from the form as:

$new_pass = trim($_POST['new_pass']);
$con_pass = trim($_POST['con_pass']);

You should not be using trim here as well. If the user wants to have space at the end/beginning of his password, your logic will fail as the user thinks his password has the space but the password you enter in the DB will not have space.

edited Nov 10 '11 at 06:49

answered Nov 10 '11 at 06:43

codaddict

445,704
82
492
529

thank you for reminding me.. when I changed my codes, then next error I get is "fail updating".. what do you think my error?? thanks again – Kevin James Nov 10 '11 at 07:01
@Kevzz There must be something wrong with your sql. Try seeing the error by doing a var_dump(mysql_error()); after the echo "fail updating"; – Jeune Nov 10 '11 at 07:12
@Jeune, thank you for giving me that code.. I knew the error of my code.. And its now working.. :D Thank a lot God bless :D – Kevin James Nov 10 '11 at 07:18

score -1 · Answer 2 · answered Nov 10 '11 at 07:17

-1

Change

if(trim('$new_pass') == trim('$con_pass'))

to

if(trim($new_pass) == trim($con_pass))

answered Nov 10 '11 at 07:17

Sonal Khunt

1,876
12
20

-1 codaddict already [said that ](http://stackoverflow.com/questions/8075820/what-wrong-with-my-php-change-password-pls-help/8075842#8075842) – yannis Nov 10 '11 at 07:20

What wrong with my php change password???

2 Answers2