patterncsharpMinor
Is this user input field secure?
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:
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:
The form is posted to the AddToCart action of the CartController:
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);
}
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:
Controller:
How it prevents CSRF:
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
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.