Dinis Cruz
Dinis Cruz

Reputation: 4289

Security vulnerability created by ASP.NET MVC ModelBinder

as I ask in detail at Can you spot the security implications/vulnerability of a small change to an ASP.NET MVC 3.0+ Model Binder? one of the versions of the CartModelBinder class (shown below) allows exploitation via MVC ModelBinding Vulnerability (also called OverPosting)

Can you spot which one?

Ideally you should provide your answer/results/proof using UnitTests :)

Version 1: Using DefaultModelBinder and CreateModel

public class CartModelBinder : DefaultModelBinder
{
    private const string sessionKey = "Cart";

    protected override object CreateModel(ControllerContext controllerContext, ModelBindingContext bindingContext, Type modelType)
    {
        // get the Cart from the session
        Cart cart = (Cart)controllerContext.HttpContext.Session[sessionKey];
        // create the Cart if there wasn't one in the session data
        if (cart == null)
        {
            cart = new Cart();
            controllerContext.HttpContext.Session[sessionKey] = cart;
        }
        // return the cart
        return cart;
    }
}

Version 2: Using IModelBinder and BindModel

public class CartModelBinder : IModelBinder
{
    private const string sessionKey = "Cart";

    public object BindModel(ControllerContext controllerContext,ModelBindingContext bindingContext)
    {

        // get the Cart from the session
        Cart cart = (Cart)controllerContext.HttpContext.Session[sessionKey];
        // create the Cart if there wasn't one in the session data
        if (cart == null)
        {
            cart = new Cart();
            controllerContext.HttpContext.Session[sessionKey] = cart;
        }
        // return the cart
        return cart;
    }
}

Controller example:

public RedirectToRouteResult AddToCart(Cart cart, int productId, string returnUrl)
{
    Product product = repository.Products
        .FirstOrDefault(p => p.ProductID == productId);

    if (product != null)
    {
        cart.AddItem(product, 1);
    }
    return RedirectToAction("Index", new { returnUrl });
}

Upvotes: 1

Views: 1946

Answers (1)

Kyle
Kyle

Reputation: 1172

Your current design can be easily misused like you suggested. A better solution would be to get the cart initially and use that instance.

  public class CartController : Controller
  {
        private IProductRepository repository;
        private IOrderProcessor orderProcessor;
        private cart;
        public CartController(IProductRepository repo, IOrderProcessor proc)
        {
            repository = repo;
            orderProcessor = proc;
            cart = Session["Cart"]; // or Cart.Current
        }

        public RedirectToRouteResult AddToCart(int productId, string returnUrl)
        {
            Product product = repository.Products
                .FirstOrDefault(p => p.ProductID == productId);

            if (product != null)
            {
                cart.AddItem(product, 1);
            }
            return RedirectToAction("Index", new { returnUrl });
        }

  }

Upvotes: 1

Related Questions