:::: MENU ::::

Tuesday, May 26, 2020

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:

View of the generated HTML

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:

Normal post

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:

Malicious post with overposting

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 (AppUseris 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 (