Skip to main content

Insecure functionality - Password management

Need​

Secure password management

Context​

  • Requirement of PHP 7.0 or later for running the application
  • Usage of Request for making HTTP requests
  • Usage of the User dependency for user-related functionality
  • Usage of Hash for secure password storage and authentication

Description​

Non compliant code​

public function changePassword(Request $request)
{
    $user_id = $request->input('user_id');
    $new_password = $request->input('new_password');

    $user = User::find($user_id);
    $user->password = Hash::make($new_password);
    $user->save();

    return redirect()->back();
}

The above code is a method in a controller that changes the password of a user. The method is vulnerable because it allows the password of any user to be changed from any other user's session. This happens because the user_id is taken from the request input, and there is no validation to ensure that the user making the request is the same user whose password is being changed.

Specifically, the vulnerability exists in these lines of code:

$user_id = $request->input('user_id');
$new_password = $request->input('new_password');

$user = User::find($user_id);
$user->password = Hash::make($new_password);
$user->save();

Here, the user_id is taken from the request input, which can be manipulated by an attacker. The password for the user with the given user_id is then changed to a new password, also taken from the request input. This means that an attacker can change the password of any user simply by sending a request with the user_id of the target user and a new password.

This is a serious security vulnerability as it allows for unauthorized access to user accounts, which could lead to data theft, unauthorized actions, and other security issues.

Steps​

  • Add authentication check to ensure the current user is the one trying to change the password.
  • Use session management to verify the user's identity before allowing password changes.
  • Implement authorization checks to prevent a user from changing another user's password.
  • Ensure the new password meets complexity requirements to enhance security.
  • Consider adding a confirmation step where the user has to input their current password before changing to a new one.

Compliant code​

public function changePassword(Request $request)
{
    $user_id = $request->input('user_id');
    $new_password = $request->input('new_password');
    $current_password = $request->input('current_password');

    $user = User::find($user_id);

    // Check if the user is authenticated and the user id from the session matches the one from the request
    if (Auth::check() && Auth::user()->id == $user_id) {
        // Check if the current password is correct
        if (Hash::check($current_password, $user->password)) {
            // Change the password
            $user->password = Hash::make($new_password);
            $user->save();
            return redirect()->back()->with('status', 'Password changed successfully');
        } else {
            return redirect()->back()->with('error', 'Current password is incorrect');
        }
    } else {
        return redirect()->back()->with('error', 'Unauthorized request');
    }
}

The updated code now includes an authentication check to ensure that the user trying to change the password is the one currently logged in. This is done by comparing the user id from the session (obtained using Auth::user()->id) with the user id from the request.

Additionally, the code now requires the user to input their current password before they can change it to a new one. This is done by checking if the hashed version of the inputted current password matches the hashed password stored in the database for the user (using Hash::check).

If the user is not authenticated, or the user ids do not match, or the current password is incorrect, the function will redirect back with an error message. If the password change is successful, it will redirect back with a success message.

References​

Last updated on