Preventing mass assignment or over posting with Razor Pages in ASP.NET Core

Mass assignment, also known as over-posting, is an attack used on websites that use model-binding. It is used to set values on the server that a developer did not expect to be set. This is a well known attack now, and has been discussed many times before, (it was a famous attack used against GitHub some years ago). In this post I describe how to stay safe from oper posting with Razor Pages.

This post is an updated version of one I wrote several years ago, talking about over posting attacks in ASP.NET Core MVC controllers. The basic premise is exactly the same, though Razor Pages makes it much easier to do the "right" thing.

What is mass assignment?

Mass assignment occurs during the model binding phase of a Razor Pages request. It happens when a user sends data in a request that you weren't expecting to be there, and that data is used to modify state on the server.

It's easier to understand with an example. Lets imagine you have a form on your website where a user can edit their name. On the same form, you also want to display some details about the user that they shouldn't be able to edit - whether they're an admin user.

Lets imagine you have the following very simple domain model of a user:

public class AppUser
{
    public string Name { get; set; }
    public bool IsAdmin { get; set; }
}

It has three properties, but you only actually allow the user to edit the Name property - the IsAdmin property is just used to control the markup they see, by adding an "Admin" badge to the markup.

@page
@model VulnerableModel

<h2>
    Edit user
     @if (Model.CurrentUser.IsAdmin)
    {
        <span class="badge badge-primary">Admin</span>
    }
</h2>

<form method="post">
    <div class="form-group">
        <label asp-for="CurrentUser.Name"></label>
        <input asp-for="CurrentUser.Name" class="form-control" />
        <span asp-validation-for="CurrentUser.Name" class="text-danger"></span>
    </div>

    <button type="submit" class="btn btn-primary">Submit</button>
</form>

This gives a form that looks something like this:

In the above Razor Page, the CurrentUser property exposes the AppUser instance that we use to display the form correctly. The vulnerability in the Razor Page is because we're directly model-binding a domain model AppUser instance to the incoming request and using that data to update the database:

Don't use the code below, it's riddled with issues!

public class VulnerableModel : PageModel
{
    private readonly AppUserService _users;
    public VulnerableModel(AppUserService users)
    {
        _users = users;
    }

    [BindProperty] // Binds the AppUser properties directly to the request
    public AppUser CurrentUser { get; set; }

    public IActionResult OnGet(int id)
    {
        CurrentUser = _users.Get(id); // load the current user. Needs null checks etc
        return Page();
    }

    public IActionResult OnPost(int id)
    {
        if (!ModelState.IsValid)
        {
            return Page();
        }

        _users.Upsert(id, CurrentUser); // update the user with the properties provided in AppUser
        return RedirectToPage();
    }
}

On the face of it, this might seem OK - in the normal browser flow, a user can only edit the Name field. When they submit the form, only the Name field will be sent to the server. When model binding occurs on the model parameter, the IsAdmin field will be unset, and the Name will have the correct value:

However, with a simple bit of HTML manipulation, or by using Postman/Fiddler for example, a malicious user can set the IsAdmin field to true, even though you didn't render a form field for it. The model binder will dutifully bind the value to the request:

If you update your database/state with the provided IsAdmin value (as the previous Razor Page does) then you have just fallen victim to mass assignment/over posting!

There's a very simple way to solve this with Razor Pages, and thankfully, it's pretty much the default approach for Razor Pages.

Using a dedicated InputModel to prevent over posting

The solution to this problem is actually very commonly known, and comes down to this: use a dedicated InputModel.

Instead of model-binding to the domain model AppUser class that contains the IsAdmin property, create a dedicated InputModel that contains only the properties that you want to bind in your form. This is commonly defined as a nested class in the Razor Page where it's used.

With this approach, we can update the Razor Page as follows:

public class SafeModel : PageModel
{
    private readonly AppUserService _users;
    public SafeModel(AppUserService users)
    {
        _users = users;
    }

    [BindProperty]
    public InputModel Input { get; set; } // Only this property is model bound
    public AppUser CurrentUser { get; set; } // NOT model bound

    public IActionResult OnGet(int id)
    {
        CurrentUser = _users.Get(id); // Needs null checks etc
        Input = new InputModel { Name = CurrentUser.Name }; // Create an InputModel from the AppUser
        return Page();
    }

    public IActionResult OnPost(int id)
    {
        if (!ModelState.IsValid)
        {
            CurrentUser = _users.Get(id); // Need to re-set properties that weren't model bound
            return Page();
        }

        var user = _users.Get(id);
        user.Name = Input.Name; // Only update the properties that have changed
        _users.Upsert(id, user);
        return RedirectToPage();
    }

    // Only properties on this nested class will be model bound
    public class InputModel
    {
        public string Name { get; set; }
    }
}

We then update the Razor Page slightly, so that the form inputs bind to the Input property, instead of CurrentUser:

 <div class="form-group">
    <label asp-for="Input.Name"></label>
    <input asp-for="Input.Name" class="form-control" />
    <span asp-validation-for="Input.Name" class="text-danger"></span>
</div>

There's a few things to note with this solution:

1. In the example above, we still have access to the same AppUser object in the view as we did before, so we can achieve exactly the same functionality (i.e. display the IsAdmin badge).
2. Only the Input property is model bound, so malicious users can only set properties that exist on the InputModel
3. We have to "re-populate" values in the OnPost that weren't model bound. In practical terms this was required for correctness previously too, I just ignored it…
4. To set values on our "domain" AppUser object, we rely on "manual" left-right copying from the InputModel to the AppUser before you save it.

Overall, there's essentially no down-sides to this approach. The only additional work you have to do is define the nested class InputModel, and also copy the values from the input to the domain object, but I'd argue they're not really downsides.

First, the nested InputModel isn't strictly necessary. In this very simple example, it's pretty much redundant, as it only has a single property, which could be set directly on the PageModel instead. If you prefer, you could do this:

public class SafeModel : PageModel
{
    [BindProperty]
    public string Name { get; set; }
}

In practice though, your InputModel will likely contain many properties, potentially with multiple data annotation attributes for validation etc. I really like having all that encapsulated in a nested class. It also simplifies the PageModel overall and makes all your pages consistent, as every page has just a single bound property called Input of type PAGENAME.InputModel. Also, being a nested class, I don't have to jump around in the file system, so there's no real overhead there either.

The final point, having to copy values back and forth between your InputModel and your domain object (AppUser) is a bit annoying. But there's not really anything you can do about that. Code like that has to exist somewhere in your application, and you already know it can't be in the model binder! You can potentially use tools like AutoMapper to automate some of this.

Another approach, which keeps separate Input and Output models is using a mediator. With this approach, the request is directly model-bound to a "command" which is dispatched to a mediator for handling. This command is the "input" model. The response from the mediator serves as the output model.

Using a separate InputModel like this really is the canonical way to avoid over-posting in Razor Pages, but I think it's interesting to consider why this approach didn't seem to be as prevalent with MVC.

Defending against over posting in MVC

In my previous post on over posting in ASP.NET Core MVC, I described multiple different ways to protect yourself from this sort of attack, many of which used extra features of the model binder to "ignore" the IsAdmin property. This typically involves adding extra attributes, like [Bind], [BindNever], or [ModelMetadataType] to convince the model binder to ignore the IsAdmin field.

The simplest option, and the best in my (and others) opinion, is simply to use separate input and output models for MVC too. The "Output" model would contain the IsAdmin and Name properties, so can render the view as before. The "Input" model would only contain the Name property, so isn't vulnerable to over posting, just as for Razor Pages.

public class InputModel
{
    public string Name { get; set; }
}

So if the answer is as simple as that, why isn't in more popular?

To be clear, it is very popular, especially if you're using the Mediator pattern with something like MediatR. I really mean why isn't it the default in all sample code for example?

As far as I can tell, the reason that separate Input/Output models wasn't more popular stems from several things:

1. The C# convention of a separate file per class. Even the small overhead of creating another file can be enough to discourage good practices!
2. The "default" MVC layout. Storing Controller, View, and Models files separately in a project, means lots of jumping around the file system. Coupled with the separate-file convention, that's just more overhead. Feature slices are designed to avoid this problem.
3. Properties on the Output model must be model-bound to the equivalent properties on the Input model. That means properties on the Input model must be named exactly the same as those on the Output model that are used to render the view. Similarly, validation metadata must be kept in-sync between the models.
4. The perceived additional left-right copying between models required. I say perceived, because once you close the over-posting vulnerability you realistically have to have some left-right copying somewhere, it just wasn't always as obvious!

These minor annoyances all add up in MVC which seems to discourage the "separate input/output" model best practice. So why didn't that happen for Razor Pages?

Razor Pages inherently tackles the first 2 points, by co-locating handlers, models, and views. It's hard to overstate just how beneficial this is compared to separate MVC views and controllers, but you really have to try it to believe it!

Point 3 above could be tackled in MVC either by using inheritance, by using separate "metadata" classes, or by using composition. Razor Pages favours the composition approach, where the InputModel is composed with the other properties required to render the view on the PageModel (CurrentUser in my previous example). This neatly side-steps many of the issues with using composition, and just fits really well into the Razor Pages model.

Point 4 is still there for Razor Pages, but as I mentioned, it's pretty much a fact of life. The only way around that is to bind directly to domain models, which you should never do, even if the ASP.NET Core getting started code does it!😱

Bonus: over posting protection != authorization

Before we finish, I just want to address a point that always seems to come up when discussing over posting:

You could edit the id parameter to update the name for a different user. How does separate-models protect against that?

The short answer: it doesn't. But it's not trying to.

The Razor Page I described above allows anyone to edit the name of any AppUser - you just need to provide a valid ID in the URL. We can't easily remove the ID from the URL, or prevent users from sending it, as we need to know which user to edit the name for. There's only really 3 feasible approaches:

• Store the ID in state on the server-side. Now you've got a whole different set of problems to manage!
• Encrypt the ID and echo it back in the request. Again, way more complex than you need, and if done incorrectly can be a security hole, or not offer the protection you think it does.
• Verify a user is authorized to edit the name. There are well-established patterns for resource-based authorization.

The final point there is clearly the correct approach to take. Before you accept a POST request that edits the name of a user, verify that the authenticated user is authorized to make that change! There's no need for some sort of custom approach - ASP.NET Core has support for imperative resource-based authorization out of the box. I also have a (rather old now) post on creating custom authorization handlers, and the source code for this post includes a basic example.

Summary

In this post I discussed mass assignment attacks, and how they work on a Razor Pages application. I then showed how to avoid the attack, by creating a nested InputModel in your Razor Page, and only using BindProperty on this single type. This keeps your vulnerable surface-area very explicit, while not exposing other values that you might need to display the Razor view correctly (i.e. IsAdmin).

This approach is pretty standard for Razor Pages, but it wasn't as easy to fall into the pit of success for MVC. The overall design of Razor Pages helps to counteract the impediments, so if you haven't already, I strongly suggest trying them out.

Finally I discussed an issue that comes up a lot that conflates over-posting with more general authorization. These are two very different topics - you can still be vulnerable to over-posting even if you have authorization, and vice-versa. In general, resource-based authorization is a good approach for tackling this side-issue.

Whatever you do, don't bind directly to your EntityFramework domain models. Pretty please.