HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Is this user input field secure?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisfieldsecureuserinput

Problem

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)
    
}


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);
}

Solution

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:

  • In the view a hidden input field with a unique string is created.



  • A cookie is sendt to the user with the same string. (I believe it is a httpOnly cookie).



  • 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.

Code Snippets

@using (Html.BeginForm("AddToCart", "Cart"))
{
   @HtmlHelper.AntiForgeryToken()
   // Code -->
}
[HttpPost][ValidateAntiForgeryToken]
public ActionResult AddToCart(int id, int quantity)
{
   // Code -->
}

Context

StackExchange Code Review Q#18286, answer score: 2

Revisions (0)

No revisions yet.