Troy Goode

MVC Controller Action Security Hole

The latest of Stephen Walther's invaluable ASP.Net MVC Tip series points out a MVC scenario that was previously unknown to me: passing cookies and server variables into controllers as action parameters. While the idea is neat, a comment left by Francois Ward echoed my immediate skepticism over whether this could be safe. After playing around I believe I have confirmed my suspicions that making use of this capability is a Very Bad Idea.

I'll let Francois' comment explain the problem (emphasis mine):

Tuesday, July 08, 2008 4:16 PM by Francois Ward Hmm, I didn't look into it much, but is that -safe-? I mean, if the variables in the index function are filled up automatically... it would be ok if it was only one type (all cookies, or all server variables), but since you can mix and match, **whats to present me from forging a request with a cookie with the same name as the server variables**? I mean, it is already possible to forge anything client-related, for obvious reason... but **forging info that potentially come from the server**? That seems...awkward... (again, keep in mind this is just my first reaction, maybe if I think it through I'll realize what I just said is totally stupid :) )

Like Francois said, cookies are easily forged client-side anyway, but most developers tend to rely on the truthiness of our server variables, specifically those that don't come over in the HTTP request. Let me demonstrate how the issue plays out.

For our example I have created a method on the HomeController named Test that takes four parameters that match server variable names. Below are the descriptions of each from MSDN's IIS Server Variables article:

My biggest concern is LOGON_USER. As described by MSDN this variable normally stores whatever the value of REMOTEUSER, UNMAPPEDREMOTEUSER, or AUTHUSER is, except when you have a third party authentication filter installed. The purpose of these authentication filters is to map the value of one of the above three request variables to a different local name. For example, you may be using a filter to map "DOMAINTroyGoode" to "DOMAIN-DMZStandardUser" and to map "DOMAINScottGuthrie" to "DOMAIN-DMZAdministrator". If you are using such a filter and then add LOGON_USER as a parameter to one of your actions, you are suddenly opening up the ability for anyone to circumvent that authentication filter.

Here is the action I have created:

And here is what this action will output without any fiddling:


Now I'll tweak the Url a bit:

First Url: http://localhost:63260/Home/Test Second Url: http://localhost:63260/Home/Test?REMOTE_ADDR=Any.IP.I.Want&LOGON_USER=YourDomainAdministrator&REQUEST_METHOD=POST&

And voilà, mischief achieved:


Now I'm no Kevin Mitnick, but I can assure you that if I can come up with something like this in an hour or so, a dedicated hacker that is probably far smarter than I will likely give you a lot of heartache if you make use of this feature. Have any "developer mode checks" that check for or localhost? Have any filters to try and prevent GETs on certain actions? Relying on server variables passed in to your actions would make those scenarios (and many others) unwise. Just say no.

In my opinion this feature (the server variable portion) should just be removed from the MVC framework entirely or something should be put in place to prevent overwriting parameters named the same as a server variable.