Changing md5 user password

[webweever]

I'm trying to write a script that changes my user passwords. I store the passwords in the DB as a md5 hash. My code is below, I keep getting error that the original username and password do not match. I require that the original credentials match so I can verify that I'm changing the password for the correct user.

 

I suspect my problem is here but I'm not sure:

$result = mysql_query("SELECT password FROM $tbl_name WHERE username='$username' and password = '".md5($pass)."'");

 

<?php
$username = check_input($_POST['username']);
$password = check_input($_POST['password']);
$newpassword = check_input($_POST['newpassword']);
$confirmpassword = check_input($_POST['confirmpassword']);

if (!isset($_POST['submit'])) { // if page is not submitted to itself echo the form
} else {
$result = mysql_query("SELECT password FROM $tbl_name WHERE username='$username' and password = '".md5($pass)."'");
if(mysql_num_rows($result)){
    if($newpassword==$confirmpassword){
        $sql=mysql_query("UPDATE $tbl_name SET password='$newpassword' where username='$username'");        
        if($sql) 
        { 
                echo "Password Changed";
        }
        else
        {
            // In case when problem while updating your new password
           echo "Error changing password, please email webmaster@mydomain.com";
        }       
    } else {
        // In case when new-password and retype-password do not match
        echo "New and confirmed password do not match please try again.";
    }
} else {
    // In case of you have not correct User name and password
	echo "Current username and password do no match."; 	
}
}
?> 


<div class="pageContent">
    <div id="main">
	<div class="container">
	<h1></h1>
               <h2>More text goes here.</h2>
	</div>
    
	<div class="container">
	<!-- All protected data goes in here -->
		<?php
			if($_SESSION['id']){
			echo '<form action="" method="post">';
			echo '<h2>Username: </h2><input type="text" name="username" size="50" maxlength="255"><br/>';
			echo '<h2>Password: </h2><input type="text" name="password" size="50" maxlength="255"><br/>';
			echo '<h2>New Password: </h2><input type="text" name="newpassword" size="50" maxlength="255"><br/>';
			echo '<h2>Confirm Password: </h2><input type="text" name="confirmpassword" size="50" maxlength="255"><br/>';
			echo '<input type="submit" name="submit" value="Change Password">';
			echo '</form>';
			} else {
			echo '<h1>Please, <a href="index.php">login</a> and come back later!</h1>';
			}
		?>
	<!-- End: All protected data goes in here -->
	</div>
        
      <div class="container tutorial-info">
      Footer goes here.  
  </div>
    </div>
</div>

 

Any ideas?

[drisate]

Juste a fast look and your using the var $password in your code

$password = check_input($_POST['password']);

 

But $pass un your MySQL querry. Was that an error?

[Muddy_Funster]

also, $tbl_name isn't defined

[cyberRobot]

Are you using mysql_real_escape_string(); maybe that's one thing that check_input() does?

http://php.net/manual/en/function.mysql-real-escape-string.php

 

If not, it would be fairly easy to change someone's (or everyone's) password with a quick SQL injection.

[webweever]

Good catches, it's always great to have another set of eyes. I fixed the table name and the $password and still get the same error.

 

mysql_num_rows(): supplied argument is not a valid MySQL result resource in myaccount.php on line 175

Current username and password do no match.

 

Line 175 is:

if(mysql_num_rows($result)){

 

<?php
$username = check_input($_POST['username']);
$password = check_input($_POST['password']);
$newpassword = check_input($_POST['newpassword']);
$confirmpassword = check_input($_POST['confirmpassword']);

if (!isset($_POST['submit'])) { // if page is not submitted to itself echo the form
} else {
$result = mysql_query("SELECT password FROM members WHERE username='$username' and password = '".md5($password)."'");
if(mysql_num_rows($result)){
    if($newpassword==$confirmpassword){
        $sql=mysql_query("UPDATE members SET password='$newpassword' where username='$username'");        
        if($sql) 
        { 
                echo "Password Changed";
        }
        else
        {
            // In case when problem while updating your new password
           echo "Error changing password, please email webmaster@signalwarrant.com";
        }       
    } else {
        // In case when new-password and retype-password do not match
        echo "New and confirmed password do not match please try again.";
    }
} else {
    // In case of you have not correct User name and password
	echo "Current username and password do no match."; 	
}
}
?> 

 

That check_input function for my form looks like this:

 

function check_input($data)
{
    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
}

 

Could it be the function that's causing the problem and which is more secure; escape strings or the function I'm using?

[btherl]

The first mysql query is causing an error - you'll need to print out mysql_error() to see why it's failing.  For example:

 

$result = mysql_query("SELECT password FROM members WHERE username='$username' and password = '".md5($password)."'") or die("Query failed: " . mysql_error());

 

Even better is if you can put the query into a variable first, and print out the query itself when displaying the error.

[webweever]

I'm an Idiot... i had the column names wrong. Also had to md5 the new password. Thanks everyone for the help

 

Aside from this I am interested in opinions on the function I'm using to check the form inputs which is more secure or is there a difference?

 

Below is what works if anyone is interested.

 

<?php
$username = check_input($_POST['username']);
$password = check_input($_POST['password']);
$newpassword = check_input($_POST['newpassword']);
$confirmpassword = check_input($_POST['confirmpassword']);

if (!isset($_POST['submit'])) { // if page is not submitted to itself echo the form
} else {
$result = mysql_query("SELECT pass FROM members WHERE usr='$username' and pass = '".md5($password)."'")or die("Query failed: " . mysql_error());
if(mysql_num_rows($result)){
    if($newpassword==$confirmpassword){
        $sql=mysql_query("UPDATE members SET pass='".md5($newpassword)."' where usr='$username'");        
        if($sql) 
        { 
                echo "Password Changed";
        }
        else
        {
            // In case when problem while updating your new password
           echo "Error changing password, please email webmaster@signalwarrant.com";
        }       
    } else {
        // In case when new-password and retype-password do not match
        echo "New and confirmed password do not match please try again.";
    }
} else {
    // In case of you have not correct User name and password
	echo "Current username and password do no match."; 	
}
}
?> 

<div class="pageContent">
    <div id="main">
	<div class="container">

	</div>
    
	<div class="container">

		<?php
			if($_SESSION['id']){
			echo '<form action="" method="post">';
			echo '<h2>Username: </h2><input type="text" name="username" size="50" maxlength="255"><br/>';
			echo '<h2>Password: </h2><input type="text" name="password" size="50" maxlength="255"><br/>';
			echo '<h2>New Password: </h2><input type="text" name="newpassword" size="50" maxlength="255"><br/>';
			echo '<h2>Confirm Password: </h2><input type="text" name="confirmpassword" size="50" maxlength="255"><br/>';
			echo '<input type="submit" name="submit" value="Change Password">';
			echo '</form>';
			} else {
			echo '<h1>Please, <a href="index.php">login</a> and come back later!</h1>';
			}
		?>
	</div>

[btherl]

function check_input($data)
{
    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
}

 

trim() is fine, assuming you don't want whitespace at the start and end.

stripslashes() is only required if your server has the magic quotes option on.

htmlspecialchars() should not be used here - it should be used just before outputting data in an HTML page.

mysql_real_escape_string() should be used, as the final step before the data goes into a MySQL query.

 

If you already have existing passwords in your database that use that processing then you may have trouble changing it, as the passwords may not match afterwards.  That's something to consider.

[btherl]

Here's the usual processing I use (more or less, I actually use Smarty for the HTML and Postgres for my database)

 

User data => trim => validate character set => Internal data

Internal data => mysql_real_escape_string => A mysql query

Internal data => htmlspecialchars => An HTML page

 

"Internal data" is the form I store the data in internally.  If it goes into mysql it gets mysql escaped, and if it goes into HTML it gets HTML escaped.  And if it goes to some other destination like XML then it gets XML escaped, etc etc.

[Drummin]

Now I run password through m5 like this before query.  Is this a good practice?

$link = mysqli_connect($host, $login, $pass);
$user=mysqli_real_escape_string($link, $_POST['username']);
$mdpass=MD5($_POST['pass1']);

$query = "SELECT ID,level FROM ".$conf['tbl']['users']." where pass='$mdpass' AND user=\"$user\"";   $result = mysql_query($query);
   $query_data = mysql_fetch_row($result);

      IF (!$query_data[0]) { 
      $error = "You have submited an incorrect login and password combination.  Please try again....";
      }

   ELSE {

[btherl]

Drummin, that looks fine from a security viewpoint.  The output of md5() never contains dangerous characters so it doesn't need to be escaped.

 

You might want to check if your query fails though.

[Drummin]

Login has been working fine, I just wondered if it looked secure.  Thank you for the reply.