ASP.NET MVC Model Binding Security

Model binding is the concept of taking values passed in through a browser request and binding them to C# objects/variables. If you're familiar with MVC then the following action method should look familiar to you; it is a trimmed down version of the action method here on CodeTunnel that adds a new PostComment to the database when a reader submits a comment.

public ActionResult AddComment(PostComment newComment)
{
    newComment.PostedDate = DateTime.Now;
    if (ModelState.IsValid)
    {
        postCommentRepository.AddPostComment(newComment);
        postCommentRepository.SaveChanges();
        return RedirectToAction("Post", new { Id = newComment.PostID });
    }
    else
    {
        // Return invalidated view
    }
}

What is happening here?

This action method expects an object of type PostComment to be supplied. This is accomplished through the use of model binding. MVC's default model binder knows how to take the values from the form post collection, parse them out, and assign them to properties of the PostComment object. This makes things super easy in the action method as I don't have to worry about parsing the form post collection myself.

What's wrong with it?

You may not realize it, but this action method as it stands currently is VERY dangerous! MVC's model binder does not discriminate; it takes all values in the form collection and if it finds values that can be bound to the PostComment then it will bind them. Be very wary of this functionality. What this means is that if a sneaky hacker wanted to spoof a request to this action method, he/she could supply their own values in the form post. Here is an example of a standard post, with form data, that arrives at the server if you post a comment using the form below:

PostID: 78
Author: Alex Ford
Email: Alex@CodeTunnel.com
Website: http://www.CodeTunnel.com
Body: This is a comment on a blog post!

This form data would be parsed by the model binder and bound to the PostComment object in the action method. But what happens if I put on my sneaky hacker hat and I tweak the request being made so that it looks like this:

PostID: 78
Author: Alex ford
Email: Alex@CodeTunnel.com
Website: http://www.CodeTunnel.com
Body: This is a comment on a blog post!
BlogPost.Body: Would you like to buy viagra for a low low price? Go to http://www.buyviagraforalowlowprice.com now!

Because CodeTunnel uses Entity Framework my entities have access to other entities via navigation properties. The problem is that, while this is extremely convenient for me, it's also extremely convenient for the model binder and the hacker. PostComment has a navigation property to the BlogPost that it belongs to. By passing in a form value called "BlogPost.Body" the hacker could bind values to the properties of the actual blog post! Or worse, what if he provided a value like this:

BlogPost.Author.Password: IAmL33tHax0r

A malicious user could potentially destroy an application because of one simple security hole.

So how do we prevent it?

In order to prevent this sort of attack there are several ways you can prevent visitors from binding to unwanted properties.

White-list Approach

This is not the most elegant solution, but it is the fastest. Simply attach a Bind attribute to the argument you are accepting.

public ActionResult AddComment(
    [Bind(Include="PostID,Author,Email,Website,Body")]PostComment newComment)
{
    //...
}

This tells the model binder to only bind those properties that are listed. Likewise you could use Exclude instead of Include to black-list certain properties from being bound. While this does solve the security problem I still don't like it for two reasons: 1) you have to write out the properties you are including/excluding in strings (gross) and 2) you have to add this attribute to every action method that binds to PostComment.

Binding Filter Approach

This solution is the one I prefer to use. I found this technique somewhere on the web but it had no formal name. I decided to call them "binding filters". The idea is that you create an interface containing only the properties you wish to bind to in a specific scenario. Then you make your object being bound inherit from these interfaces. Finally we explicitly call the model binder, passing in our interface, and let it work its magic.

First our interface:

/// <summary>
/// Binding filter declaring only the properties I want bound.
/// </summary>
public interface INewPostComment
{
    string Author { get; set; }
    string Website { get; set; }
    string Email { get; set; }
    string Body { get; set; }
    int PostID { get; set; }
}

/// <summary>
/// Partial class to extend EF generated entity. Inherits from binding filters.
/// </summary>
public partial class PostComment : INewPostComment
{
    //...
}

/// <summary>
/// Explicitly call model binding within the action method using TryUpdateModel.
/// </summary>
public ActionResult AddComment()
{
    PostComment newComment = new PostComment();

    // Use the <T> to pass in the interface type. The model binder will only bind to those properties defined on the interface,
    // and update newComment appropriately.
    TryUpdateModel<INewPostComment>(newComment);
    newComment.PostedDate = DateTime.Now;
    if (ModelState.IsValid)
    {
        postCommentRepository.AddPostComment(newComment);
        postCommentRepository.SaveChanges();
        return RedirectToAction("Post", new { Id = newComment.PostID });
    }
    else
    {
        // Return invalidated view
    }
}

It is more work than throwing up a quick Bind attribute but it builds a reusable foundation that can be used again and again. This will save you time in the long run. On top of that, it's strongly-typed. If properties on your entity ever change then you will get a compilation error saying that your entity no longer implements your binding filters appropriately and you can fix them accordingly. This is a great way to ensure that only the properties you want bound get bound!

Another benefit of this is that you can type your views to the binding filter interface. This makes your views only aware of the properties on the interface. This eliminates the need to type the TryUpdateModel() method as well.

View

@model IBlogPostFilter
@using(Html.BeginForm())
{
    <text>
        @Html.EditorForModel()
        <input type="submit" value="Submit" />
    </text>
}

Controller

[HttpPost]
public ActionResult NewBlogPost()
{
	IBlogPostFilter model = new BlogPost();
	TryUpdateModel(model);
}

You no longer need to pass a type in TryUpdateModel() because the type is already of IBlogPostFilter.

Interface Binding Filter vs View Model Binding

A reader named Zootius poses an interesting question in the comments. It is something I neglected to mention in this post. There is one other common way to perform model binding security through the use of a view model.

A view model is a simple model class that is used specifically for a view, only containing the properties that the view will need and be allowed to bind to. So instead of typing my AddComment view to the type PostComment I would type it to something like PostCommentViewModel.

public class PostCommentViewModel
{
    public string Author { get; set; }
    public string Website { get; set; }
    public string Email { get; set; }
    string Body { get; set; }
    public int ID { get; set; }
}

Once I have my view model I would then not have to worry about security in the model binder as it only has access to the properties of the view model. This approach is powerful but there are a few reasons why I prefer to use interface binding filters over the view model if I can.

Firstly, I have to do some tedious data transfer. If I want to pre-populate a form with the data from an existing post comment I would have to do something like this:

public ActionResult EditComment(int id)
{
    PostComment existingComment = postCommentRepository.GetPostComment(id);
    PostCommentViewModel viewModel = new PostCommentViewModel
    {
        Author = existingComment.Author,
        Website = existingComment.Website,
        Email = existingComment.Email,
        Body = existingComment.Body,
        Id = existingComment.Id
    }
    return View(viewModel);
}

This is rather tedious when I could just pass my existing model object if I was using interface binding filters. I have to do the same thing again when this form posts back with the view model:

public ActionResult EditComment(PostCommentViewModel viewModel)
{
    PostComment existingComment = postCommentRepository.GetPostComment(viewModel.Id);
    existingComment.Author = viewModel.Author;
    existingComment.Website = viewModel.Website;
    existingComment.Email = viewModel.Email;
    existingComment.Body = viewModel.Body;
    // ...
}

This is again rather tedious. Despite the extra work involved view models are still the most widely used technique for ensuring that model binding is secure. There is however, another reason why I do not like to use them as much. Model validation.

Model validation allows you to decorate properties on your model with validation attributes (data annotations). These validation attributes go over the top of whatever properties the model binder is going to bind to. Using the view model method I would have to do something like this:

public class PostCommentViewModel
{
    [Required(ErrorMessage="You must include your name.")]
    public string Author { get; set; }

    public string Website { get; set; }

    [Required(ErrorMessage="You must include your email.")]
    [ValidEmail(ErrorMessage="Not a valid email.")] // This is a custom validation attribute.
    public string Email { get; set; }

    [Required(ErrorMessage="You must include a body.")]
    string Body { get; set; }

    public int ID { get; set; }
}

This works great as the model binder will see these attributes and add validation errors to the ModelState dictionary and the MVC framework knows how to display those when you re-render the view with errors. Let's pretend now that an administrator has the ability to edit post comments and approve or disapprove of them using an admin form. I now suddenly need an entirely new view model with all the same validation attributes duplicated.

public class PostCommentAdminViewModel
{
    [Required(ErrorMessage="You must include your name.")]
    public string Author { get; set; }

    public string Website { get; set; }

    [Required(ErrorMessage="You must include your email.")]
    [ValidEmail(ErrorMessage="Not a valid email.")] // This is a custom validation attribute.
    public string Email { get; set; }

    [Required(ErrorMessage="You must include a body.")]
    string Body { get; set; }

    public int ID { get; set; }

    public bool Approved { get; set; }
}

All that just to add one new property to an administrator version of the same form. That is really laborious and redundant. Model validation attributes can be added to my actual PostComment model. If we then use interface binding filters we could just create a new interface binding filter without worrying about model validation being duplicated:

public interface INewPostComment
{
    string Author { get; set; }
    string Website { get; set; }
    string Email { get; set; }
    string Body { get; set; }
    int PostID { get; set; }
}

That's it. You're done! I like this so much better than duplicating major chunks of view models. That said, there are instances where you cannot type your view to one model object and you instead need various different pieces of data. In these instances you have to use a view model in order to give your view access to all that it needs. However, using interface binding filters this is super easy! Most MVCers would tell you off for creating a view model like this:

public class SomeViewModel
{
    public BlogPost BlogPost { get; set; }
    public string LoggedInUsername { get; set; }
    // etc...
}

This would normally be a bad view model because it's only a tiny layer of abstraction over your real domain model objects. You would still have security holes with the BlogPost and PostComments properties. All you have to do is modify your view model to contain the binding filter interface as a property:

public class SomeViewModel
{
    public IBlogPostFilter BlogPost { get; set; }
    public string LoggedInUsername { get; set; }
    // etc...
}

Now you are free to pass SomeViewModel to your view and use all of it's properties to your heart's content, knowing that if/when you post any of it back for model binding the binding filter interface will take effect. No more security holes and you still did not have to repeat any model validation attributes on your view model :D

public ActionResult SomeActionMethod()
{
    SomeViewModel viewModel = new SomeViewModel
    {
        BlogPost = new BlogPost(),
    }
    TryUpdateModel(viewModel);
}

Note that you have to new up the BlogPost property in my example. That is because it is an interface now. The model binder would not know how to instantiate that property when you call TryUpdateModel() so we must instantiate it with the implementing class we desire. After that you can call TryUpdateModel(). Because the model it is trying to update is of type SomeViewModel and the BlogPost property on that model is of type IBlogPostFilter the model binder will only see the properties defined in the filter. The other nice part of this is even when you first load the view that is typed to SomeViewModel the view will only have access to the filtered properties as well. This completely hides the properties you did not want to be seen/used.