Insecure object reference - User deletion

Need

Secure user deletion process and session management

Context

• Usage of C# 7.1 for developing applications with advanced language features
• Usage of Microsoft.AspNetCore.Mvc for building web applications with ASP.NET Core MVC
• Usage of Microsoft.EntityFrameworkCore for object-relational mapping (ORM) in .NET applications

Description

Non compliant code

[HttpDelete("{id}")]
public async Task<IActionResult> DeleteUser(int id)
{
    var user = await _context.Users.FindAsync(id);
    if (user == null)
    {
        return NotFound();
    }

    _context.Users.Remove(user);
    await _context.SaveChangesAsync();

    return NoContent();
}

The above code snippet is a typical implementation of a DELETE operation in an ASP.NET Core application. This function allows any user to delete any other user just by knowing their ID. This is a severe security issue as an attacker could delete all users in the system, making it inoperative.

Furthermore, the system does not invalidate the session of the deleted user. This means that if a user is deleted while they are still logged in, they can continue to interact with the system, which can lead to unpredictable behavior or further security issues.

The vulnerability lies in the fact that there are no checks to see if the user requesting the deletion has the right to do so. In a secure system, only an administrator or the user themselves (after re-authentication) should be able to delete a user account.

Moreover, once a user is deleted, their session should be immediately invalidated, forcing them to log out. This is not happening in the current implementation, which is another part of the vulnerability.

In order to fix this vulnerability, we need to add authorization checks to ensure that only authorized users can delete accounts, and we need to invalidate the session of the deleted user.

Steps

• Implement authentication and authorization mechanisms to ensure that only authorized users can delete other users.
• Validate the user's identity and permissions before allowing the deletion operation.
• Ensure that the user's session is invalidated and terminated upon deletion to prevent unauthorized access.
• Consider implementing a soft delete mechanism instead of permanently removing users from the system, allowing for easier recovery and auditing of user data.

Compliant code

[Authorize(Roles = "Admin")]
[HttpDelete("{id}")]
public async Task<IActionResult> DeleteUser(int id)
{
    var user = await _context.Users.FindAsync(id);
    if (user == null)
    {
        return NotFound();
    }

    // Check if the current user is the same as the one to be deleted
    var currentUserId = User.FindFirst(ClaimTypes.NameIdentifier)?.Value;
    if (currentUserId == id.ToString())
    {
        // Invalidate the user's session
        await HttpContext.SignOutAsync();
    }

    // Implement soft delete
    user.IsDeleted = true;
    _context.Users.Update(user);
    await _context.SaveChangesAsync();

    return NoContent();
}

The updated code now includes several security measures to address the identified vulnerability:

1. Authorization: The Authorize attribute is used to ensure that only users with the "Admin" role can delete other users. This prevents unauthorized users from deleting users.

2. User Validation: The code checks if the current user is the same as the one to be deleted. This is done by comparing the NameIdentifier claim of the current user with the id of the user to be deleted.

3. Session Invalidation: If the current user is the same as the one to be deleted, the user's session is invalidated using HttpContext.SignOutAsync(). This ensures that the user cannot continue browsing after deletion.

4. Soft Delete: Instead of permanently removing the user from the system, a soft delete mechanism is implemented. The IsDeleted property of the user is set to true and the user is updated in the context. This allows for easier recovery and auditing of user data.

References

• 369.Insecure object reference - User deletion