ASP.NET MVC - Think Before You Bind

I don't know about most of you out there, but I know that I am extremely excited about the impending release of ASP.NET MVC. I'm even more curious though about what kind of adoption we are going to start seeing out of the gate, especially being that companies have invested so much money in developers learning ASP.NET Web Forms. There is one thing that could stand in the way of adoption, and that is horror stories coming from early adopters about security issues or flaws in production web applications that were overlooked because developers didn't have to think as much about these kinds of issues in ASP.NET Web Forms.

Most of these issues revolve around escaping output that is going into the HTML and dealing with post data manually. Something that I have been looking at recently is the model binding abilities that ASP.NET MVC provides us. In case you aren't familiar with what I am talking about, it is now possible to tell ASP.NET to bind a class on an action method using a default model binder. So let's say we have a controller action with a signature like this:

[AcceptVerbs("POST")]  
public ActionResult Edit(Product product)

How does ASP.NET MVC know what to do with the post data that is coming to this method? Well, it uses the default model binder and it tries to bind any post data that matches the patters of "parameterName.PropertyName". So if on my form I have a field called "product.Name" then ASP.NET MVC will bind the value of this field to the product class that is coming into the "Edit" action. It just so happens that this is both convenient and very dangerous.

The problems come in when you consider the idea of post data tampering. What if I created a form to update my product that only had the ability to edit the Name, Price, and a few other attributes of the product? And let's say that I am a malicious person who loves tools like the "Tamper Data" Firefox plugin. Which is actually quite the useful tool for development.

The first thing that we will do is to fire up "Tamper Data".

Then we will tell it to start tampering:

Now, when we post back the form, we get this nice little form that looks like this:

And we can right click and add a new item. So what would happen if we added an item like this?

Hmmmm. Of course ASP.NET MVC has no way to know that you don't want this property updated, and it also has no way of knowing that the post data is invalid. Remember, we don't have any viewstate, or serverside state of any kind that would let us know what fields are posting back from the form.

This is a pretty major problem if you are just blindly binding data on the server side. So, what do you do? Well, there are a few options actually.

The first, and probably most obvious is to just pull the data yourself and copy it to your object:

[AcceptVerbs("POST")]  
public ActionResult Edit(FormCollection form)

This is pretty inefficient, but you get complete control. As you see above, if you put a "FormCollection" object in the parameter list then ASP.NET MVC will bind the requests form collection to it. The second method is to use "UpdateModel":

UpdateModel(product, new[] { "ProductName", "UnitPrice", "Discontinued", "ReorderLevel" });

Here we are calling the UpdateModel method on the controller, and we are passing in a list of properties to bind instead of just letting it bind everything. In order to do this, we would just retreive the product from the database and let this method update it, or create a new product and pass it into this method. You wouldn't have the Product class itself in the parameters to the action.

Another option would be to create interfaces for the properties that you want updated, and then call "UpdateModel" using an instance of the interface instead of the class type, this way you would only get the correct properties bound, without the risk of binding properties that you don't want.

You could also create your own model binder for products that would only bind the properties that you wanted. This is a bit more limiting though, since you would either have to specify the model binder globally (which means you can't limit which properties are bound differently in different places) or you have to specify the model binder using attributes on every action parameter that uses the class. Talk about ugly!

If you wanted to bind globally, you could do this in the global.asax:

ModelBinders.Binders[typeof(Product)] = new ProductBinder();

Otherwise it'll look like this:

[AcceptVerbs("POST")]  
public ActionResult Edit([ModelBinder(typeof(ProductBinder))] Product product)

A final option is one that was suggested to me by Simone Chiaretta (who referenced Jeremy Miller's work) is to create a presentation only model. This way you could define a presentation model which allowed you to only bind the properties you needed. This would still be an issue though if you were looking to bind different properties from different forms, you would still have to exploit one of the methods above.

Update: Sorry, I left off one of the most obvious ways of accomplishing this as well, just putting parameters on the action that match the name of the fields on the form. So, if we had "ProductName" and "UnitPrice" we would have an action that looked like this:

[AcceptVerbs("POST")]  
public ActionResult Edit(string ProductName, string UnitPrice)

This way we can just parse out the individual items instead of using the "FormCollection".

If you have any other suggestions on ways to accomplish this, please let me know! I'm looking for ideas, links, etc... I would really love to see ASP.NET MVC succeed, and educating the public on ways to avoid some of these pitfalls is one way that we can do this. If you have a blog, and work with ASP.NET MVC, I encourage you to get the word out there regarding the new pitfalls that ASP.NET developers will face when moving to MVC.