Search code examples
c#jqueryajaxasp.net-mvcstack-overflow

C# AntiForgeryToken attribute causes StackOverflowException in mvc application


I have created a antiforgery attribute class to decorate my GenericBaseController class:

[AttributeUsage(AttributeTargets.Class)]
public class ValidateAntiForgeryTokenAttribute : AuthorizeAttribute
{
    public override void OnAuthorization(AuthorizationContext filterContext)
    {
        var request = filterContext.HttpContext.Request;

        //  Only validate POSTs
        if (request.HttpMethod == WebRequestMethods.Http.Post)
        {
            //  Ajax POSTs and normal form posts have to be treated differently when it comes
            //  to validating the AntiForgeryToken
            if (request.IsAjaxRequest())
            {
                var antiForgeryCookie = request.Cookies[AntiForgeryConfig.CookieName];

                var cookieValue = antiForgeryCookie != null
                    ? antiForgeryCookie.Value
                    : null;

                AntiForgery.Validate(cookieValue, request.Headers["__RequestVerificationToken"]);
            }
            else
            {
                new ValidateAntiForgeryTokenAttribute()
                    .OnAuthorization(filterContext);
            }
        }
    }
}

(reference link http://richiban.uk/2013/02/06/validating-net-mvc-4-anti-forgery-tokens-in-ajax-requests/ )

once a normal POST call in application is done (not ajax), I always get a StackOverflowException. Application without ValidateAntiForgeryTokenAttribute works fine. If I debug the code inside this class, after a post request, flow keeps going trough the line

new ValidateAntiForgeryTokenAttribute()
    .OnAuthorization(filterContext);

infinitely. People in linked article assure that this implementation works, so I'm wondering why I'm getting this problem. Is it really supposed to create a new ValidateAntiForgeryTokenAttribute when the request is not ajax ?


Solution

  • Boiled down to the problem, your code is:

    public class ValidateAntiForgeryTokenAttribute : AuthorizeAttribute
    {
        public override void OnAuthorization(AuthorizationContext filterContext)
        {
            if ( evaluateCondition() )
            {}
            else
            {
                new ValidateAntiForgeryTokenAttribute()
                    .OnAuthorization(filterContext);
            }
        }
    }
    

    The problem

    Your call is recursive in the else block:

    • The class you are calling the method on is ValidateAntiForgeryTokenAttribute.

    • In your else block you have

      new ValidateAntiForgeryTokenAttribute()
          .OnAuthorization(filterContext);
      

      which, given that the calling method is

      public override void OnAuthorization(AuthorizationContext filterContext)
      

      means that you will keep calling OnAuthorization (i.e. the same method) on new instances of a ValidateAntiForgeryTokenAttribute.


    Solution

    In the example you posted, the situation was slightly different - the name of the class is ValidateAntiForgeryTokenOnAllPosts whereas yours is ValidateAntiForgeryTokenAttribute, so the call is not recursive since the method is not calling itself with the same arguments.

    You have three options - I'm not sure which is best for your situation (I'm thinking the first one):

    • Change your Attribute name to ValidateAntiForgeryTokenOnAllPosts to match the name in the example you posted.

    • Explicitly state that you want System.Web.Mvc.ValidateAntiForgeryTokenAttribute by changing the block to say

      new System.Web.Mvc.ValidateAntiForgeryTokenAttribute()
          .OnAuthorization(filterContext);
      
    • Since you are overriding ValidateAntiForgeryTokenAttribute, you can call the base method, i.e.

      else
      {
          base.OnAuthorization(filterContext);
      }