Skip to content

Instantly share code, notes, and snippets.

@rgranadino
Created June 17, 2015 14:00
Show Gist options
  • Save rgranadino/97611298b195d335164c to your computer and use it in GitHub Desktop.
Save rgranadino/97611298b195d335164c to your computer and use it in GitHub Desktop.
csrf draft

I believe it will be difficult to source a definitive answer to this question of why a CSRF token is "needed" in Magento's add to cart GET action. I'll make an attempt to interpret its purpose. I'm by no means a security expert and this is my interpretation of CSRF in this particular context.

Context

From owasp.org

Cross-Site Request Forgery (CSRF) is an attack that forces an end user to execute unwanted actions on a web application in which they're currently authenticated. CSRF attacks specifically target state-changing requests, not theft of data, since the attacker has no way to see the response to the forged request.

One example of this attack is embedding a hidden image in an email or an alternate webpage:

<img src="http://shop.com/cart/add?sku=sprocket&qty=5" width="0" height="0" border="0">

The web server would not differentiate where the request came from and would faithfully add the item to that user's cart.

The goal of preventing CSRF attacks is to prevent state-changing requests. Adding an item to a cart would be considered as change in state. In general, I would consider this is a harmless state change compared to submitting an order, transferring funds or updating an email address.

Regarding state changes and HTTP methods, RFC 2616 says the following:

In particular, the convention has been established that the GET and HEAD methods SHOULD NOT have the significance of taking an action other than retrieval. These methods ought to be considered "safe".

Magento and CSRF

Magento does implement CSRF prevention mechanism for both admin and frontend areas by using a token (form key). I would assume that Magento's goal, as a platform meant to be built upon by other developers, is to secure all state changing requests. The reason being that they have no idea what the implementing developers or 3rd party extensions may inadvertently expose. It's safer to secure all state changing requests than to have something exposed by a 3rd party module and be bad PR for the platform. I don't actually know if all state changing requests are secured from CSRF attacks.

One thing to note is that Magento does not always use a form key to protect state changing requests. For instance, the Delete From Cart (/checkout/cart/delete/id/{ID}) and Delete Customer Address (/customer/address/delete/id/{ID}) requests are both GET requests that pass in an Entity ID. The controller (or models) then handle ensuring that the user is authorized to remove those entity records (modify state). These are two instances where Magento doesn't follow RFC 2616. To be fair, for some use cases it might not be practical or necessary to do so.

It appears that the form key check in the Mage_Checkout_CartController::addAction method was added in version 1.8. From the release notes we have the following to go off of:

Resolved issues that could have resulted in Cross-Site Request Forgery (CSRF) in the web store.

Unfortunately the language is vague and the "could have" leads me to believe it was due to the assumption I stated earlier: secure state changing requests. There might be a possibility of sending additional parameters which cause some sort of unintended behavior: /checkout/cart/add/product/337/email/new@address.com/password/l33tp4ssw0rd

The idea being that by adding something to the cart there's some bit of code (core or 3rd party) which happens to be triggered during add to cart, e.g. via some event dispatched.

It seems unlikely that such a vulnerability exists out of the box and if it does one would hope Magento would do a better job of disclosing the details/risks. One risk I could see is that Magento blindly stores the request parameters during the add to cart in the product_options column of the sales order items table (see: info_buyRequest). In theory someone could trick a group of users into adding items to their cart with weird query parameters and that would get stored into sales_flat_quote_item_option table and eventually the sales_flat_order_item table if they're also able to get those users to convert. This seems high unlikely to me in most cases.

Add to Cart Form Key Issues

One of the big issues people run into with a FPC implementation and CSRF tokens is they end up getting cached. The first client that warms the cache generates the token, when the second client gets a cache HIT they now have been given a page with the first users token. When submitting the form the tokens wont match.

Magento Enterprise uses placeholders to find/replace the form keys in cached pages. Varnish implementations will likley use an ESI to wherever a form key is used.

I would be curious to know whether some the more popular "ajax cart" extensions end up checking the CSRF token during their add to cart requests.

The one feature request where I end up foregoing the CSRF token for the add to cart action is allowing the ability to create add to cart links for use in emails or other web sites (social media). Sometimes marketing wants to have users directly add an item to the cart and redirect to the cart or checkout immediately. This can't easily be done if a CSRF token is required. I'd only recommend this if you're comfortable with the level of risk and trust your own and any 3rd party code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment