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:

1
2
3
4
5
6

// GET: /Product/Edit/1
public IActionResult Edit(int? id)
{
    // id value is 1
    // rest of the code
}

It can also be more complicated, for example we can bind a bunch of GET parameters into custom model:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13

public class Page
{
    public int CurrentPage { get; set; }
    public int PageSize { get; set; }
}

// GET: /Products?CurrentPage=2&PageSize=5
public IActionResult Index(Page page)
{
    // page.CurrentPage value is 2
    // page.PageSize value is 5
    // rest of the code
}

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:

1
2
3
4
5
6

public class User
{
    public string Login { get; set; }
    public string Password { get; set; }
    public string Role { get; set; }
}

This is the view with password change form:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14

@model User

You're @Model.Role
<form asp-action="Edit" asp-Controller="User">
<div class="form-group">
<label asp-for="Login"></label>
<input class="form-control" type="text" asp-for="Login" />
</div>
<div class="form-group">
<label asp-for="Password"></label>
<input class="form-control" type="text" asp-for="Password" />
</div>
<button class="btn btn-sm" type="submit">Save</button>
</form>

And this is how our action looks like:

1
2
3
4
5
6
7
8

// Normally it should be a POST request
// but I've used GET to make it easier
// GET: /Edit?Login=user&Password=123456
public IActionResult Edit(User user)
{
    _context.Update(user);
    return View(user);
}

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:

1

// GET: /Edit?Login=user&Password=123456&Role=admin

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:

1
2
3
4
5
6
7

// GET: /Edit?Login=user&Password=123456
public IActionResult Edit(User user)
{
    user.Role = "regular";
    _context.Update(user);
    return View(user);
}

BindAttribute

One way to prevent binding of unwated properties is to use [Bind] attribute on model.

We can use it to pick only bindable properties:

1
2
3
4
5

public IActionResult Register([Bind("Login", "Password")]User user)
{
    // user.Role is null
    return View();
}

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. Login to 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):

1
2
3
4

public IActionResult Register([Bind(nameof(User.Login), nameof(User.Password))]User user)
{
    return View();
}

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:

1
2
3
4
5
6
7

public class User
{
    public string Login { get; set; }
    public string Password { get; set; }
    [BindNever]
    public string Role { get; set; }
}

Most appropriate in my opinion would be BindNever, the other one seems to not be appropriate in this case.

TryUpdateModelAsync

Controllers have a neat little method called TryUpdateModelAsync which helps us update only specified fields:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

public async Task<IActionResult> Edit()
{
    // retrieve current user (e.g. from database)
    var user = GetCurrentUser();
    // update specified fields based on providers data
    if (await TryUpdateModelAsync<User>(user,
        "",
        c => c.Login, c => c.Password))
    {
        try
        {
            await _context.SaveChangesAsync();
        }
        catch (DbUpdateException)
        {
            // Log the error
        }

        return RedirectToAction("Index");
    }
    
    return View();
}

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.

1
2
3
4
5

public class UserViewModel
{
    public string Login { get; set; }
    public string Password { get; set; }
}

1
2
3
4
5

public IActionResult Edit(UserViewModel userViewModel)
{
    var user = Map(userViewModel).To<User>();
    return View();
}

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 [FromBody]!

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.

1
2
3
4
5

public IActionResult Register([FromBody]User user)
{
    // user.Role could be set with JSON request
    return View();
}

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.

Summary

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.