-1

I have a form to change the password of user. It takes the input on profile page and then sends to another page for processing. But i am getting the error login_failed even if i am leaving any of the field blnk. In blank case, the error should be Fill all the fields. Can someone tell where is the problem.

<form method="post" action="updatepassword.php">
           <input name="pass" type="text" placeholder="Old password" /> 
           <input name="pass1" type="text" placeholder="New password" /> 
           <input name="pass2" type="text" placeholder="Confirm password" />
                      <input id="butt" type="submit" value="Update" name="pinchange"> 
                      </form>

updatepassword.php

<?php
include_once("login_status.php");

}

if(isset($_POST['pinchange']))
{
 $pass=md5($_POST['pass']);
 $pass1=md5($_POST['pass1']);
 $pass2=md5($_POST['pass2']);
 if($pass==""||$pass1==""||$pass2==""){ echo "Fill all the fields"; exit();}
 else{
 $sql = "SELECT id,password FROM users WHERE username='$log_username' ;
        $query = mysqli_query($db_conx, $sql);
        $row = mysqli_fetch_row($query);
        $db_id = $row[0];
        $db_pass_str = $row[1];
        if($pass != $db_pass_str){      
            echo "login_failed";
            exit();
        }
}

}
?>
  • What happens if the username is wrong? – Álvaro González Jul 26 '14 at 11:40
  • Don't use md5, please don't. just don't. really, trusy me. don't. MD5 was never designed for password hasing. – Pinoniq Jul 26 '14 at 11:53
  • @Pinoniq can you explain why not to use md5..?? – bhushya Jul 26 '14 at 11:58
  • google my friend: http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords md5 is an algorithm designed for speed. not security – Pinoniq Jul 26 '14 at 12:01
  • @Pinoniq: I kow this too my friend that it is not safe for hashing. I am working on a dummy project. But i am about to complete it and now my worry is the hashing technique. I searched alot for this but still no good tutorial found. Everyone tells how to change using different techniques but no one tells how to authenticate. – Shailendra Hunt Arya Jul 26 '14 at 12:12
  • @ Álvaro G. Vicario : I am including the login_status.php in beginning which will provide the username. – Shailendra Hunt Arya Jul 26 '14 at 12:15
  • Pardon? You only have one username and it's hard-coded? – Álvaro González Jul 26 '14 at 13:06
  • Use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner Jul 26 '14 at 14:55

1 Answers1

2

IMPORTANT NOTE: it is considered very bad practice to use MD5 for storing and retrieving passwords. This answer just shows what was wrong with the code. It does not encourage the use of MD5 hashing at all.

You should only convert the passwords with md5 after you compare them to empty strings. So you should do:

<?php
include_once("login_status.php");

if(isset($_POST['pinchange']))
{
 $pass=$_POST['pass'];
 $pass1=$_POST['pass1'];
 $pass2=$_POST['pass2'];
 if($pass==""||$pass1==""||$pass2==""){ echo "Fill all the fields"; exit();}
 else{
     $sql = "SELECT id,password FROM users WHERE username='$log_username'" ;
     $query = mysqli_query($db_conx, $sql);
     $row = mysqli_fetch_row($query);
     $db_id = $row[0];
     $db_pass_str = $row[1];
     if(md5($pass) != $db_pass_str){      
         echo "login_failed";
         exit();
     }
}
lowerends
  • 5,469
  • 2
  • 24
  • 36
  • 1
    Actually, you shouldn't use md5 for passwords. Ever. – The Blue Dog Jul 26 '14 at 11:53
  • Absolutely true, but it does pinpoint the problem at hand here. – lowerends Jul 26 '14 at 11:54
  • @lowerends: Thanks alot.. it worked. One more question to ask. Right know,in cae of errors it is showing in a blank pge. I want to show it just below the form near submit button. i know it could be done using JavaScript but really dont know how. Any suggestion on it. – Shailendra Hunt Arya Jul 26 '14 at 12:09
  • I suggest you open a new question for that tagged with PHP and Javascript. That will give you the best chance for a good answer. – lowerends Jul 26 '14 at 12:29
  • Here is my final code with JavaScript. If i am supplying all the fields correctly, then please wait is coming but no further response is being received..what should i do here? – Shailendra Hunt Arya Jul 26 '14 at 14:15
  • I would have upvoted this, but not mentioning the fact that the OP stands at eventually getting hacked using a hashing method that is considered broken, and not giving proper guidance, is why. I no longer give "answers" related to MD5 password storage methods; not without adding an alternate and safer method as well as using prepared statements; it's just a question of "time". – Funk Forty Niner Jul 26 '14 at 14:43
  • @Fred -ii- I've added an explicit note to the top of my answer to make it clear to anybody reading this answer and not the comments that MD5 should not be used. – lowerends Jul 27 '14 at 07:35
  • 1
    It's always best to do so. Should you encounter more questions involving MD5 or any other unsafe hashing method, you could tell them to use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). ;-) – Funk Forty Niner Jul 27 '14 at 14:46