One of the many security risks which you should consider is a mass assignment vulnerability (cheatsheet) also know as overposting. While it’s not in OWASP Top 10 it’s still considered important. Read on to understand the issue and find out possible ways of fixing it.
Mass assignment explained
ASP.NET Core allows automatic model binding of request parameters into variables or objects to make developers life easier. It can be as simple as binding
id parameter via route:
It can also be more complicated, for example we can bind a bunch of
GET parameters into custom model:
This behavior can be harmful. Attacker can guess parameter names and overwrite variables which should remain intact.
Let’s see how this works by example. Let’s assume that we have a simple web application where users can change login and password.
This is our database model:
This is the view with password change form:
And this is how our action looks like:
If the user found out that we’re having
Role property, he could try to overwrite it and set it to “admin”.
Now, what would happen if attacker crafted a malformed request like this:
In this scenario our evil user could promote his account to admin, because there is nothing in our code to prevent that.
We may think that we’re safe because we haven’t included
Role property in our form or we made it hidden, but it doesn’t prevent proper binding.
How to deal with it?
This vulnerability happens during model binding when someone is trying to create request in a way which allows overriding model properties.
There are a couple of security countermeasures to help us prevent mass assignment.
As we’ll see there are two groups of solutions for this problem. First is to manually specify which properties should or shouldn’t be binded (whitelisting/blacklisting) using data annotations. Second is to add another layer to our application (view models).
The bad approach
While you could technically fix this problem by manually nulling/zeroing all other variables, you shouln’t follow this way. It’s error prone, verbose and we’ve better solutions available out of the box.
So I strongly advise against doing something like this:
One way to prevent binding of unwated properties is to use
[Bind] attribute on model.
We can use it to pick only bindable properties:
Other model properties would simply be ignored, so even if someone posted
Role property it wouldn’t be binded to model. We’re basically whitelisting properties here.
Unfortunately this solution uses magic strings, so if you decide to change your property name (e.g.
Username) you will have to remember and change it manually.
Pretty error prone solution if you ask me, but since we have the
nameof operator there is a way to make it a litte bit better (and more verbose):
EditableAttribute / BindNeverAttribute / ReadOnlyAtribute
Next possibility is to use one of property attributes which will prevent binding:
Those attributes has to be used directly on model fields like so:
Most appropriate in my opinion would be
BindNever, the other one seems to not be appropriate in this case.
Controllers have a neat little method called
TryUpdateModelAsync which helps us update only specified fields:
The drawback of this solution is that
TryUpdateModelAsync is tied to controller, so if you (want to) have a separate database services layer it will cause some trouble.
Also you may dislike this solution because it tends to put too much database related operations into controller.
ViewModel / Data Transfer Object
Many would say that it’s best to provide additional layer which would be responsible for data exchange between controller and view.
Separation of concerns is a good idea, after all database model is not the same as view model.
Besides introducing data transfer objects / view models layer to our app can help us prevent mass assignment vulnerabilities.
This way we have view-models with minimum required properties which are mapped to EntityFramework models afterwards.
Providing additional layer has its own drawback which is extra work. It takes some time to create additional classes and view model to model mapping, but in return we’re having a better layered application.
We can use solutions like Automapper to help us map values from view-model to database model and back.
If you’re building an (JSON) API: attributes won’t work with
Model binding using different formatters (JSON, XML, etc) won’t take standard attributes into account.
Simply model binding knows nothing about your serializer/deserializer, so regular attributes won’t apply.
You can try to check if your serializer has build-in attributes which could replace
[BindNever] e.g. Json.NET has [JsonIgnore] which could be used to prevent binding.
What you can do in this case:
- use data attributes provided by your serializer/deserializer
- write custom binder which would take attributes into account
- use data transfer object approach.
In my opinion it’s best not to bind directly to database model. Personally I avoid attribute based solutions.
I prefer to have separate view-model layer and binding directly to entity models doesn’t appeal to me. In return we have separation of concerns and we’re hiding internal (database) data structures away from users.
Though most people would probably use attribute based approach for basic stuff and view-models for advanced apps.
Don’t forget that you may need different security measures when you’re accepting JSON (or other) formatted requests.