Policing ActionResults

Wednesday, June 30, 2010

In the previous post we looked at using unit tests and reflection to ensure every controller derived from a specific base class in an ASP.NET MVC application. You might use a base class to ensure action filters are always in place for auditing, logging, or other general purpose security and runtime diagnostics.

You can also use action filters to inspect the result of a controller action. For example, you might want to check the Url of every RedirectResult to preventĀ  open redirects in your application. Spammers love sites with open redirects.

Edit: thanks, Sergio for catching a copy/paste error (update to put the throw inside the if check). Sergio also pointed out the code allows redirects to non-HTTP URIs, and as shown doesn't allow a redirect to a local path (/home/index/123).

public class VerifyRedirects : ActionFilterAttribute
{
    public override void OnResultExecuting(ResultExecutingContext filterContext)
    {
        var redirect = filterContext.Result as RedirectResult;
        if(redirect != null)
        {
            var host = new Uri(redirect.Url).Host;
            if(_whiteList.Any(host.EndsWith))
            {
                return;
            }            
            throw new InvalidOperationException(BadRedirectMessage);
        }
    }

    string[] _whiteList =
        {
            "microsoft.com", "odetocode.com", "pluralsight.com"
        };

    const string BadRedirectMessage = "...";
}

In this case we are checking the URL against a white list of known destinations. If the host isn't found, the request stops with an exception.


Comments
gravatar Justin A Thursday, July 1, 2010
So simple and yet so important :)

That should be added to MVCContrib (wink wink) :)

-J-
gravatar Scott Thursday, July 1, 2010
@Justin - The hard part is coming up with a way to supply the white list that will work for 80% of people.

Come to think of it, just restricting redirects to the same host might work for 80% of people...
gravatar Harry Steinhilber Thursday, July 1, 2010
Restricting redirects to the same host would be good, but couldn't you also just simply supply the whitelist through an attribute constructor? That would get the use cases where the hosts are known at compile time.
gravatar Scott Allen Thursday, July 1, 2010
@Harry: Ah yes, I could see that working in a lot of scenarios, too.
gravatar Thomas Freudenberg Tuesday, July 6, 2010
Instead of restricting the target of the redirct, I would check the referrer of the request. E.g. your website wants to keep track of redirects to 3rd party sites, so your pages link to /redirect?url=..., which in rturn redirects to the original link. In this case you have to check, if it was one of your own pages which initiated the call to /redirect?...
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!