Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Preventing tampering of form fields in ASP.NET MVC EF

The default strongly-typed Edit page in ASP.NET MVC 3 generally exposes all fields for the Entity. While this is often workable, some fields are a security risk. For example a simplified magazine subscription entity might look like:

public void Subscription() {
  public int Id { get; set; }
  public string Name { get; set; }
  public string Address { get; set; }
  public string City { get; set; }
  public string State { get; set; }
  public string Zip { get; set; }
  public DateTime SubscribedThru { get; set; }
}

If I provide an Edit page to let users change their own address, for example, it's a security risk to include the SubscribedThru field because a knowledgeable and malicious user could give themselves a free 10-year subscription by faking the date (even if I use @Html.HiddenFor(model => model.SubscribedThru). So I am not including that field in any way on the Edit page html (via razor).

I thought the answer might be to prevent binding attempts on SubscribedThru on the Edit method in the controller using something like:

[HttpPost]
public ActionResult Edit([Bind(Exclude="SubscribedThru")] Subscription subscription) {
  if (ModelState.IsValid) {
    db.Entry(subscription).State = EntityState.Modified;
    db.SaveChanges();
    return RedirectToAction("Index");
    }
  }
  return View(subscription);
}

When I get to the SaveChanges(); line, it throws the error The conversion of a datetime2 data type to a datetime data type resulted in an out-of-range value. I believe that the SubscribedThru date (properly?) doesn't exist, and the empty value is less than SQL Server can handle. What surprises me is that it's even trying to update that field when I have Binding excluded for it.

So far my best solution seems to be to create a custom ViewModel that omits the SubscribedThru date, but that seems a lot of duplication of fields, validation, etc.; if possible I'd like to just make the one field SubscribedThru safe from user editing.

I can't say I fully understand the UpdateModel and TryUpdateModel methods and wonder if that's a direction to head? I played with them and EF throws errors for having duplicate objects (same key) which is perplexing.

Also, I'm not clear if the subscription data is preserved from the initial load in public ActionResult Edit(int id) in the controller all the way to the final [HttpPost] public ActionResult Edit(Subscription subscription)... method, or does the line db.Entry(subscription).State = EntityState.Modified; try and set all the data (I thought it was just setting a flag indicating "edited-so-EF-should-save-this").

I'm a long-time .NET developer, just jumping in to my first ASP.NET MVC project, so I'm probably overlooking something painfully obvious. Thanks for any help!

like image 537
Larry S Avatar asked Oct 07 '22 13:10

Larry S


2 Answers

So far my best solution seems to be to create a custom ViewModel that omits the SubscribedThru date, but that seems a lot of duplication of fields, validation, etc.;

That is exactly what you should do to keep things neat & tidy. AutoMapper eases the ViewModel variation headache.

like image 119
bluevector Avatar answered Oct 12 '22 22:10

bluevector


This page contains an example of updating a model using TryUpdateModel (Listing 4): http://www.asp.net/mvc/tutorials/older-versions/models-(data)/creating-model-classes-with-the-entity-framework-cs

You can whitelist only the fields that you allow to be edited, which removes the security risk.

like image 37
Richard Avatar answered Oct 12 '22 22:10

Richard