Is this user input field secure?

Question

I'm pretty new to asp and c# and I have a couple of concerns with the security of the code I have written (shown below). Basically all it does is allow you to enter a quantity before you add the item to your cart using a razor form.

Every example I see uses validation on input fields, like:

[Required]
[DataType(DataType.EmailAddress)]

Is that sort of validation (or any other) necessary in this case, or is my form secure enough because the controller requires the input of the product id and the quantity, both of which are limited to integers? Also, is there anything else that I should be concerned about with this form, like Javascript injection?

Thanks for your help!

The view has a razor input form:

@using ( Html.BeginForm( "AddToCart", "Cart" ) )
{
    @Html.HiddenFor(_ => item .ProductID)
    @Html.TextBoxFor(_=> item.Quantity)
    <input type="submit" value="+ Add to cart" />
}

The form is posted to the AddToCart action of the CartController:

[HttpPost]
public ActionResult AddToCart(int id, int quantity)
{
    // Retriebe the product from the database
     var addedProduct = storeDB.Products
         .Single(product => product.ProductId == id);

    // Add it to the shopping cart
    var cart = ShoppingCart.GetCart(this.HttpContext);

    cart.AddToCart(addedProduct, quantity);

    //Go back to the main store page for more shopping
    return RedirectToAction("Index");
}

And here is the AddToCart method:

public void AddToCart(Product product, int quantity)
{
    //Get the matching cart and album instances
    var cartItem = storeDB.Carts.SingleOrDefault(
        c => c.CartId == ShoppingCartId
            && c.ProductId == product.ProductId);

    if (cartItem == null)
    {
        // Create a new cart item if no cart exists
        cartItem = new Cart
        {
            ProductId = product.ProductId,
            CartId = ShoppingCartId,
            Count = quantity,
            DateCreated = DateTime.Now
        };
        storeDB.Carts.Add(cartItem);
    }
    else
    {
        // If the item does exist in the car, 
        // than add one to the quantity
        var newCount = cartItem.Count + quantity;
        cartItem.Count = newCount;
    }
    // Save changes
    storeDB.SaveChanges();
}

Thanks again for your help!

Answer 1

You can secure against CSRF (pronounced C-surf) by using ValidateAntiForgeryTokenAttribute and AntiForgeryToken method.

How CSRF (Cross Site Request Forgery) works:

We have 2 actors, an evil person E and a good person G. E makes an evil html image tag on some forum, that links to your website and puts a product in your user's cart.

If G has a cookie for your site and the cookie is not expired, when G goes to E's forum, the browser tries to load the html img and it will send a Request to your site. Even though G didn't intend to do that...

Implementation:

View:

@using (Html.BeginForm("AddToCart", "Cart"))
{
   @HtmlHelper.AntiForgeryToken()
   // Code -->
}

Controller:

[HttpPost][ValidateAntiForgeryToken]
public ActionResult AddToCart(int id, int quantity)
{
   // Code -->
}

How it prevents CSRF:

1. In the view a hidden input field with a unique string is created.
2. A cookie is sendt to the user with the same string. (I believe it is a httpOnly cookie).
3. The controller checks if the form contains the same string as is in the cookie.

Because person E, cannot form a valid form post the cross site request will fail. But note that if you have XSS (Cross site scripting) vulnerability this security check can be circumvented.

And another thing... If your 2nd method public void AddToCart(Product product, int quantity) is not an action consider using the [NonAction] attribute, because all public methods in a controller are considered by default Action methods. Meaning it is invokable as an Action method.

Answer 2

Security is an important aspect of any application, so you must evaluate what you want to do for security.If this is a public portal then you need to make sure all query string value are safe and no problem with the input value , MVC also does care these things automatically but t to make sure you can double check this params.

like in this case you can make sure quantity cannot be zero or product ID cannot be zero.

It depends upon your requirement and your security model.