CODEWITHSUNDEEP
c#asp.net-mvcasp.net-core
UnworthyToast
UnworthyToast

Reputation: 825

How do I prevent an end user from modifying an object by its primary key via an overposting attack in C#/.NET?

I'm sure this is an obvious answer, but I am learning C# and .NET for the first time now. In Microsoft's tutorial for ASP.NET Core MVC, the example shows including an object's ID property in the Edit bind.

[HttpPost]
[ValidateAntiForgeryToken]
public async Task<IActionResult> Edit(int id, [Bind("ID,Title,ReleaseDate,Genre,Price")] Movie movie)
{
    if (id != movie.ID)
    {
        return NotFound();
    }
    // other stuff
}

In the view, there is also a hidden input field for the ID; I understand why this is necessary. Anyway, I followed along and verified that by finding that hidden field in my browser's HTML viewer and changing the value of the ID in that hidden field to another known valid ID, I could effectively submit changes that would be applied to an object other than the one I had initially clicked on to edit.

Is this just a mistake on Microsoft's part for including ID in the bind in their example, or am I missing something? This seems very dangerous on a production site. When I removed ID from the bind include I get a 404 when trying to submit my changes, so it's obviously not as simple as that. I assume this is because the if (id != movie.ID) check fails but I am not completely sure.

What is the solution here to allow the site to function properly (i.e. send the target ID to the controller and then to the database) without allowing a bad actor to supply their own ID?

Upvotes: 0

Views: 474

Answers (1)

Tseng
Tseng

Reputation: 64259

I mean I'm just starting to learn this stuff... I even said I haven't done authentication yet. I plan to in the future. But the issue I am describing still would not be fixed with authentication. Authentication would tell me who messed with things, but would not prevent them from doing so.

You can't solve it without authentication and authorization, neither you should try to hard. In the end, from application perspective you will never know, which entity (with what ID) was supposed to be changed.

Only thing you can check is, if the user is permitted to change it, based on your business rules for that function, for example

  • Check if user has a specific permission to edit/update that type of entities (i.e. ManageUsers policy). Such a policy already implies, the user is allowed to manage all users, so if he decides to change the ID, well so he do. He could done the same by selecting a different user end editing it. From the point of the application nothing changed

  • If the user is only allowed to edit users, belonging to a specific group, well then you need to add means to determine that, i.e. add some permissions that indicate the user is allowed to edit groups, but only specific ones (i.e. where the user has an administrator role).

    In that case, you need to load the entity (var user = context.Users.FirstOrDefault(u => u.Id == id), then check the group to what that user belongs (var groups = soemService.GetUserGroups(user)) and then check if the logged in user, has the permissions to modify this groups (bool hasAccess = authorizationService.CanEditRoles(groups, this.User) (where this.User is the logged in user from the controller's property) and if successful, then allow the change, otherwise reject it via return Forbidden() result)

There may be other logics you can apply, but it depends on your application, i.e. you may only want users to edit their own entities, in that case the entity needs to have a AuthorId property, which you check against the logged in users Id (if(user.AuthorId == loggedInUserId) { ... }).

In anyways, all this logic only works, when you have authentication. Without it, all kind of checks are pointless, because you have no means to find out what a specific user is allowed to or not.

Upvotes: 2

Related Questions