Dangers on MVC Blind Model Binding

A few weeks ago I spotted something weird while working on a .NET MVC application, once I realized it was something really dangerous I wrote a brief article to warn other experts on this matter, I made this because I've never found anything on this issue during my MVC learning. You can see this first article here

Well. Days have passed and I've been able to investigate a bit further on the matter. To be honest I must say that actually there is some information on this type of problems on the web, but it wasn't so easy to find as I'd like to for an issue like that.  I've found that there are many people out there unaware of these dangers. Usually different frameworks offer several solutions to this problems but those solutions are not activated by default and it is not normal to find a tutorial or guide that makes reference to it. I think that this article can be helpful to a bunch of people out there that are, like me just a few weeks ago, blind to the dangers of this.

One last point before entering into matter: although I'll stick to .NET MVC framework on this article, its content is applicable also to any other MVC framework that allows blind model binding (like Ruby on Rails, for example).

Well... let's start from the beginning.

What exactly is "Blind model binding"?

Model binding is a characteristic of some MVC frameworks that allow you to directly map fields on the View with fields on the Model, this way you can simply use the same naming guidelines on your forms and your database model and save lots of typing during Controller's saving proccesses.

If you work with .NET MVC framwork surely you know already that I'm talking about UpdateModel method.

Ok, Model Binding is great, it helps a lot with the development, so what happens with it's "blindness"?

The concept is, that you shouldn't use directly (blindly) model binding from your View to your Business Model, as doing this you open your entire Business Model to the client machine, where a savvy user can get your View code, alter it, and gain access to manipulate data that you don't want him to edit. This could sound plain simply for some readers but believe me, there is much people out there that don't know it or don't know why it's so important.

Let's use an example before going deeper:
Think about an intranet application for a company where users can ask for their vacation periods. Surely you'll have a Business Model Class to store the petitions from different users, and those petitions will probably have a "status" field (accepted, rejected, etc...).

On the app we'll have a form to create petitions where any user can make new petitions, and another form where HR personnel can accept or reject petitions.

Imagine this as a very simply version of our controller method for creating petitions:

[HttpPost, Authorize]
                      public ActionResult Create (FormCollection formValues) { 
                        PETITION newPetition = new PETITION();
                        if (ModelState.IsValid()) {
                          try {
                            UpdateModel(newPetition); // --> Blind Model Binding!! Danger!!
                            databaseRepository.Add(newPetition);
                            databaseRepository.Save();
                            return View("detailNewPetition");
                          } catch {
                            // Whatever
                          }
                        }
                      }
                      

Select all Open in new window

On the previous example, UpdateModel wil check all parameters on the Request your form has sent and save it directly to database. A user with enough knowledge would be able to alter data on your View and send distorted data; he could be able, for example, to populate a new form from your page's source code and add fields that you've kept away from your form, like status. Your users would be able to make petitions that would be directly approved!

Sure, it is needed some knowledge of the database model to be able to exploit this, but it's easier than you can imagine to obtain that info or simply to hit a weak spot on a trial and error basis.

So why to have potential vulnerabilities on your apps when you can simply avoid them following some simple design rules?

And the first rule on designing MVC applications is: Never trust info from client.

So, when you manage an HttpPost petition to save data check always that the user on session have permissions to do the operation and avoid copying all the data blindly to database.

Let's check a better approach to our previous controller:

[HttpPost, Authorize]
                      public ActionResult Create (FormCollection formValues) { 
                        PetitionViewModel newPetition = new PetitionViewModel(); // --> Class that protects your business model from evil users
                        if (ModelState.IsValid()) {
                          try {
                            UpdateModel(newPetition); // It's not blind because PetitionViewModel will only have those fields that are "public"
                            PETITION petition = new PETITION(newPetition);
                            databaseRepository.Add(petition);
                            databaseRepository.Save();
                            return View("detailNewPetition");
                          } catch {
                            //Whatever
                          }
                        }
                      }
                      

Select all Open in new window

What have we changed here?

We have used a new class to receive data from HttpPost: PetitionViewModel, this class is simply a wrap for those fields you WANT your uses to be able to fill on the creation form, so when the UpdateModel is executed, any other info not contained in the "secure" view model is not loaded onto that class instance, being ignored and discarded. This way a user will be no more able to add a petition with a status. Later, after data is stored on the intermediate class, you can download that info into the business model class by a specific constructor, with a method of your intermediate class or in any other way that you prefer, knowing that no extra info has been stored on the database.

Wait a minute... and what about those fields users cannot edit but I need on my view to create the business model?

Well... certainly an intermediate class is not the answer for ANY situation, you should always have your guard on when designing your controllers.

Let's take a closer look at our example:
Think that we want our application to allow HR users to create petitions for any worker on the company but, of course, any worker out of this department should only be able to create it's own petitions. In your creation form you could load a dropdown with users for the HR department users while normal users get a disabled dropdown with it's name directly picked, or better yet, they can see their name on the page but there isn't any dropdownlist or field to pick from (although you store their id on an input type=hidden).
Well, if they are sneaky enough, they can edit your source code to gain access to it's user identifier and change it, allowing them to create petitions for any user.

So, to avoid this type of behaviour we should enforce a bit more secure controller:

[HttpPost, Authorize]
                      public ActionResult Create (FormCollection formValues) { 
                        PetitionViewModel newPetition = new PetitionViewModel(); 
                        if (User.IsInRole("HRDepartment") || Guid.Parse(Request.Form["UserId"]) == database.getIdFromUserName(User.Identity.Name)) {
                          if (ModelState.IsValid()) {
                            try {
                              UpdateModel(newPetition);
                              PETITION petition = new PETITION(newPetition);
                              databaseRepository.Add(petition);
                              databaseRepository.Save();
                              return View("detailNewPetition");
                            } catch {
                              //Whatever
                            }
                          }
                        else 
                          return View("operationNotAllowed");
                      }
                      

Select all Open in new window

Here we check, before starting to do anything more, that the user in session has the appropiated role for the operation or it's user identity matches the identity of the user of the petition being created. When none of both conditions are fullfilled the controller returns to a specific view where we warn about an invalid operation.

Keep in mind that the examples on this article are deliberately simple, as my solely purpose is to show the dangers behind model binding default operative. I know there are better approaches, but here I seek to be clean and simple. I'll try to add some bibliography to the article where you can check more refinated solutions.

I hope this helps, if you came this far thank you for your time.