-1

I have created a registration system for a sample android app. I can both register and login without any problems. The last part is the forgot password one. Here is my logic. When the user forgets his password,he sends his email to my server and in return he gets an email back telling him to visit a link. That link takes him to a page where he can enter and confirm his new password. For some reason though the password is not updated.

So I will post my code here.

register.php

   <?php
     session_start();
     require "init.php";
     header('Content-type: application/json');
     $id = $_POST['id'];
     $email = $_POST['email'];
     $user_name = $_POST['user_name'];

     $user_pass = $_POST['user_pass'];
      $passwordEncrypted = sha1($user_pass);  

      $confirmPass = $_POST['confirm_pass'];
      $confPasswordEncrypted = sha1($confirmPass);  

      $msg = "Congratulations. You are now registered to the most amazing   
      app ever!";            

        if(!filter_var($email, FILTER_VALIDATE_EMAIL)){


            $don = array('result' =>"fail","message"=>"Please enter a valid email");

        }    



if($email && $user_name && $user_pass && $confirmPass && filter_var($email, FILTER_VALIDATE_EMAIL)){


    $sql_query = "select * from user_info WHERE email  ='".mysqli_real_escape_string($con, $email)."' or user_name 
    ='".mysqli_real_escape_string($con, $user_name)."'";

    $result = mysqli_query($con, $sql_query);   

    $results = mysqli_num_rows($result);

    if ($results){
        $don = array('result' =>"fail","message"=>"Email or username exists.");
    }else{

        $sql_query = "insert into user_info values('$id','$email','$user_name','$passwordEncrypted','$confPasswordEncrypted');";

        if(mysqli_query($con,$sql_query)){

            $don = array('result' =>"success","message"=>"Successfully registered!Well done");
            mail($email,"Well done. You are registered to my sample app!",$msg);
            $_SESSION['id'] = mysqli_insert_id($con);
        }
    }
}else if(!$email){


        $don = array('result' =>"fail","message"=>"Please enter a valid email");               


    }else if(!$user_name){

        $don = array('result' =>"fail","message"=>"Please enter your username");

    }else if(!$user_pass){

        $don = array('result' =>"fail","message"=>"Please enter a password");

    }else if(!confirmPass){

        $don = array('result' =>"fail","message"=>"Please confirm your password");

    }




    echo json_encode($don);

?>

And the changepassword.php

<?php
require "../init.php";
session_start();
if(isset($_POST['update'])){
    $password = $_POST['user_pass'];
    $confpassword = $_POST['confirm_pass'];
    if($password !== $confpassword){
       echo "Passwords don't match!";
    }else{
        $id = $_SESSION['id'];
        if(mysqli_query($con,"UPDATE user_info SET   
        user_pass='$password',confirm_pass = '$confpassword' WHERE id='$id'")){
            echo "Password successfully changed!!!";
        }
    }

}
?>
<!DOCTYPE HTML>
<html>
<head>
 <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
 <link rel="stylesheet" href="css/login-css.css" media="all"/>
 </head>
 <body>

 <div class="updatepass">
   <h1>Update Password</h1>
<form action="" method="post">
    <input type="password" name="user_pass" placeholder="Password" required="required" />
    <input type="password" name="confirm_pass" placeholder="Confirm Password" required="required" />
    <button type="submit" class="btn btn-primary btn-block btn-large" name="update">Update</button>
</form>
</div>

</body>

</html>

Any ideas on how to update the password?

Thanks.

Theo
  • 3,099
  • 12
  • 53
  • 94
  • 3
    Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure that you [don't escape passwords](http://stackoverflow.com/q/36628418/1011527) or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Apr 21 '16 at 16:05
  • 3
    [Little Bobby](http://bobby-tables.com/) says [your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Apr 21 '16 at 16:06
  • You don't need `confirm_pass` in your DB. Check them in your script then just store the password (if they match). – chris85 Apr 21 '16 at 16:08
  • 4
    Have you checked your error logs? You're making an assumption the query is working. Add error reporting to the top of your file(s) right after your opening ` – Jay Blanchard Apr 21 '16 at 16:08
  • I am aware of SQL injection. For the moment I want to update the password. After fixing that I will hash the updated password as you see in register.php. – Theo Apr 21 '16 at 16:09
  • Are `changepassword.php` and `register.php` in the same directory? I ask because your path to `init.php` is different in each script. What errors are you receiving (if any)? You need to find out where the script is failing. You aren't allowing for any proper debugging. What if `if($password !== $confpassword)` fails, but so does the query. At least check your query by adding an `else { echo mysqli_error($con); }` to `if(mysqli_query($con, ...)` to see if any errors are returning. – mferly Apr 21 '16 at 16:10
  • 1
    If you're aware of the SQL injection bugs but refuse to fix them it shows you just don't care about doing it correctly. Computers are a harsh judge, and those that try and hack into them are even more cruel. Do it right the first time. Don't make excuses. [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) is not hard to use and will save you tons of headaches since it always escapes correctly. – tadman Apr 21 '16 at 16:13
  • @Marcus nope, they are not in the same directory. – Theo Apr 21 '16 at 16:14
  • Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.2/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and never store passwords without a proper password hash applied. – tadman Apr 21 '16 at 16:18
  • @Theo: Avoid `require` and `require_once` whenever you can, classes + autoloaders go a long way. Also: `set_include_path` makes life a lot easier when you find yourself `require`'ing files: you don't have to go through the pains of working out (and repairing) all those relative paths all the time – Elias Van Ootegem Apr 21 '16 at 16:19
  • *"After fixing that I will hash the updated password as you see in register.php"* - sha1 and `password_hash()` have different lengths, so make sure that if and when you do switch to the latter, that the column length is minimum 60. – Funk Forty Niner Apr 21 '16 at 16:19
  • I think that the $id=$_SESSION['id'] is not working. – Theo Apr 21 '16 at 16:20
  • @Theo do as already suggested and check for errors, you're not doing that. Use a conditional `isset()` or `!empty()` against all arrays and make sure that whitespace isn't being introduced. `var_dump();` is something else you can use to debug. `if(mysqli_query($con,"UPDATE user_info SET user_pass='$password',confirm_pass = '$confpassword' WHERE id='$id'")){ echo "Password successfully changed!!!"; } else { echo "Something failed and you need to find out why."; }` – Funk Forty Niner Apr 21 '16 at 16:22
  • var_dump($id) under the line $id=$_SESSION['id'] gives me null. Meaning that there is nothing there. – Theo Apr 21 '16 at 16:25
  • there you go @Theo your session array is empty and your query depends on it. The session array never gets set. – Funk Forty Niner Apr 21 '16 at 16:26
  • @ Fred -ii. But I set it in register.php. look at line $_SESSION['id'] = mysqli_insert_id($con); Also i have session_start() inside the forgotpassword.php. So I don't understand why my session array is not set! – Theo Apr 21 '16 at 16:29
  • @Theo the problem here is that you're relying on that session from another file to register. Once you get them to reset their password, the session's been lost. What you need to do is to query / SELECT in the same code as your password reset file. – Funk Forty Niner Apr 21 '16 at 16:31
  • @Theo also, what you need to do is to include a reference to the mail sent that queries their username/email address, used with a GET array and encode their email if it's used as the principal method of verification and sent back to the verification process and not use session arrays for this. That's the best I can offer at this time without having to do a full rewrite. – Funk Forty Niner Apr 21 '16 at 16:35

2 Answers2

1

I did it!!! As Fred -ii- suggested I had to do a select query in order to select the id. So.

<?php
require "../init.php";
session_start();

if(isset($_POST['update'])){
    $password = $_POST['user_pass'];
    $confpassword = $_POST['confirm_pass'];
    if($password !== $confpassword){
       echo "Passwords don't match!";
    }else{
        $select_query = "SELECT id FROM user_info";
        $run_select_query = mysqli_query($con,$select_query); 
        while ($row_post=mysqli_fetch_array($run_select_query)){

             $user_id = $row_post['id']; 

             echo $user_id;

        }

        $update_posts = "UPDATE user_info SET user_pass='$password',confirm_pass = '$confpassword' WHERE id='$user_id'";  
        $run_update = mysqli_query($con,$update_posts); 
        echo "<script>alert('Post Has been Updated!')</script>";
    }

}
?>

But again I want to leave the password "unprotected" because I want to learn how sql injection works:):). If you find anything else please comment..

Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
Theo
  • 3,099
  • 12
  • 53
  • 94
  • 1
    Your logic seems a bit off. Most sites would generate a random long UNIQUE string , then save it to database (users table) temporarily (24 hours or so) then send you a link containing the string. Something like http://example.com/reset.php?string=qwe9019012391-239-soadi. Then they would look in their users table for the string SELECT * FROM users WHERE password_reset_token='qwe9019012391-239-soadi' and then use it to update the user. It's the accepted way to do things like that. Not trough sessions :) and on SQL injection.. there is a ton of tutorials around the net. No need to explain here. – Hop hop Apr 21 '16 at 16:52
  • 1
    @YavorKirov 's suggestion is a good one Theo. I myself use a table where unique tokens are stored (from generated) and then deleted from it once the user has verified from a link sent. – Funk Forty Niner Apr 21 '16 at 16:56
  • Leaving the *"password unprotected"* != testing SQL injection. – Jay Blanchard Apr 21 '16 at 16:59
  • @Fred -ii-. Yes. The password now is updated. However,my android doesn't let me to to log in:(. When I register for the first time,I can login. The problem appears after updating the password. – Theo Apr 21 '16 at 17:04
0

Edited:

Here's my take on it:

your problem seems to be with

$_SESSION['id'];

you are setting it only when creating account but PHP Sessions don't persist for too long. So if your user is registered and you store the row id in session, you can't count on it being there when they return to change their password.

This is (probably) what happens behind the scenes when you try to find your row to update the password

http://prntscr.com/av3320

Hop hop
  • 841
  • 8
  • 21