0

Hello I have a script for validating the current password before updating it. It works but when I tried to update the email is shows Undefined variable: pass but in the other forms it is not even showing that error. How to fix?

Here is the html code.

<?php
       $user_email = $_SESSION['user_email'];
     $sql = "SELECT * FROM users WHERE user_email = '$user_email'";
     $query = $conn->query($sql);
     $row = $query->fetch_array();
    //  echo '<pre>' . var_export($_SESSION, true) . '</pre>';
    ?>
<div class="form-group">
       <label class="d-flex justify-content-center"><strong>Update Information</strong></label>
             <label>Fullname</label>
             <input type="text" name="user_fullname" value="<?php echo $row['user_fullname']; ?>" class="form-control" placeholder="" required>
                </div>
       <div class="form-group">
             <label>Email</label>
             <input type="email" name="user_email" value="<?php echo $row['user_email']; ?>" class="form-control" placeholder="" required>
             </div>
       <div class="form-group">
             <label>Current Password</label>
             <input type="password" name="curpassword" class="form-control" pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}" 
        title="Must contain at least one number and one uppercase and lowercase letter, and at least 8 or more characters"
        placeholder="Enter Current Password" required>
             </div>
             <div class="form-group">
             <label>Password</label>
             <input type="password" name="password" class="form-control" pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}" 
        title="Must contain at least one number and one uppercase and lowercase letter, and at least 8 or more characters"
        placeholder="Enter New Password" required>
       <input type="hidden" name="binder" value="<?php echo $user_email; ?>">
             </div>
       <div class="form-group">
             <label>Confirm Password</label>
             <input type="password" name="cpassword" class="form-control" pattern="(?=.*\d)(?=.*[a-z])(?=.*[A-Z]).{8,}" 
        title="Must contain at least one number and one uppercase and lowercase letter, and at least 8 or more characters"
        placeholder="Confirm Password" required>
             </div>
             <input type="submit" name="submitna" value="Update" class="btn btn-success">
              </form>
             </div>
             <div class="modal-footer">
            <button data-dismiss="modal" class="btn btn-primary" type="button">Close</button>
         </div>

Here is the php script.

if(isset($_POST['submitna'])){
  $binder = $_POST['binder'];
  $user_fullname = $_POST['user_fullname'];
  $user_email = $_POST['user_email'];
  $curpassword = md5($_POST['curpassword']);
  $password = md5($_POST['password']);
  $cpassword = md5($_POST['cpassword']);

$sql = "SELECT * FROM users WHERE user_email = '$user_email' ";
$query = $conn->query($sql);
while($row = $query->fetch_array()){
$pass = $row['password'];
}
if($curpassword != $pass){
  echo '<script>alert("Incorrect Current Password")

  </script>';
}else if($password !== $cpassword){
  echo '<script>alert("Password not matched")
  window.location = "index.php"
  </script>';
}else{
  $sql="UPDATE users SET user_fullname=?, user_email=?, password=? WHERE user_email=?";
  $stmt=$conn->prepare($sql);
  $stmt->bind_param("ssss", $user_fullname, $user_email, $password, $binder);
  if($stmt->execute()){
    $_SESSION['user_email'] = $user_email;
    echo '<script>alert("Information has been updated")
          window.location = "index.php"
          </script>';
  }
}

}

The error only shows when I tried to change the email. What seems to be the cause of it?

  • You're overwriting `$pass` each time through the loop, so you only get the value from the last row. If the query only returns one row, why are you looping? – Barmar Oct 23 '21 at 04:11
  • Ah what should I do? – Kaido Bagua Oct 23 '21 at 04:12
  • **Never store passwords in clear text or using MD5/SHA1!** Only store password hashes created using PHP's [`password_hash()`](https://php.net/manual/en/function.password-hash.php), which you can then verify using [`password_verify()`](https://php.net/manual/en/function.password-verify.php). Take a look at this post: [How to use password_hash](https://stackoverflow.com/q/30279321/1839439) and learn more about [bcrypt & password hashing in PHP](https://stackoverflow.com/a/6337021/1839439) – Dharman Oct 23 '21 at 13:25
  • **Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Oct 23 '21 at 13:25

2 Answers2

1

If there's no matching email, the while loop that fetches the results will never execute, so you'll never set $pass.

You need to check that the query returned a row of results before trying to use the variables you set from fetching.

f(isset($_POST['submitna'])){
    $binder = $_POST['binder'];
    $user_fullname = $_POST['user_fullname'];
    $user_email = $_POST['user_email'];
    $curpassword = md5($_POST['curpassword']);
    $password = md5($_POST['password']);

    if ($_POST['password'] != $_POST['cpassword']) {
        echo '<script>alert("Password not matched");window.location = "index.php"</script>';
        exit;
    }

    $sql = "SELECT * FROM users WHERE user_email = ?";
    $stmt = $conn->prepare($sql);
    $stmt->bind_param("s", $binder);
    $stmt->execute();
    $result = $stmt->get_result();
    $row = $result->fetch_array();
    if (!$row) {
        echo '<script>alert("Incorrect username")</script>';
    } else {
        $pass = $row['password'];
        if($curpassword != $pass){
            echo '<script>alert("Incorrect Current Password")</script>';
            exit;
        } else {
            $sql="UPDATE users SET user_fullname=?, user_email=?, password=? WHERE user_email=?";
            $stmt=$conn->prepare($sql);
            $stmt->bind_param("ssss", $user_fullname, $user_email, $password, $binder);
            if($stmt->execute()){
                $_SESSION['user_email'] = $user_email;
                echo '<script>alert("Information has been updated")
          window.location = "index.php"
          </script>';
            }
        }
    }
}

Other points:

  • Compare the plaintext password and password confirmation with each other, not their hashes and not with the database. If the confirmation doesn't match, there's no need to do anything with the database.
  • Use prepared statements whenever a query contains user input.
  • MD5 is not a secure password hash function. I didn't show it here, but you should use password_hash() and password_verify().
  • You should look up the old password using $binder, not $user_email, just like you do in the UPDATE query. $binder is their old email from the hidden input, $user_email is the new email they're changing it to.
Barmar
  • 741,623
  • 53
  • 500
  • 612
-1

Your $pass variable is being created and set to the result row 'password' from this query: $sql = "SELECT * FROM users WHERE user_email = '$user_email' ";

Is the row 'password' in fact being returned by that query? If not, then $pass is not defined.

  • How can I define it? – Kaido Bagua Oct 23 '21 at 04:10
  • Because it echo incorrect current password even though it's correct before showing that error only when I changed the email. – Kaido Bagua Oct 23 '21 at 04:12
  • @KaidoBagua The query has to return at least one row. If there's no row that matches `user_email = '$user_email'` the `while` loop will never run and you don't set `$pass`. – Barmar Oct 23 '21 at 04:12