Using the QueryString as a debug switch?!

Question

Hi,

I was refactoring some code in a web application today and came across something like this in the base class for all webpages:

if (Request.QueryString["IgnoreValidation"] != null)
{
    if (Request.QueryString["IgnoreValidation"].ToUpper() == "TRUE")
    {
        SessionData.IgnoreValidation = true;
    }
}

To me, this appears to be a Very Bad Thing™, so I instantly removed all traces of this flag from the code. For one, there were several if statements littered throughout that checked the value of the flag, leading to cluttered and unclear logic. Secondly, I came across another, more dangerous flag named "IgnoreCreditCardValidation". You can guess what that one did...

I then got to thinking about it and remembered a similar example from a previous job. In the code of an app sold as a "secure authentication module" there was a QueryString parameter used to override the default behavior, effectively allowing anyone with knowledge of it to bypass authentication.

Now my question is more of a confirmation, is this practice as bad as it seems in my head or am I just overreacting and this is commonplace? Are there any cases where there would be a valid reason to do this? To me it just seems like an awful mix of laziness and carelessness.

If this is a duplicate, please feel free to point me in the right direction.

Thanks!

Answer 1

It's horrifying whether it's common practice or not. +1 to you for nuking it with extreme prejudice.

Answer 2

I agree with you. Especially if the module is designed to enforce security, this is a stupid thing to have in a release build (it's not a good idea to have in debug builds either, but that might be reasonable.) It's essentially security-by-obscurity.

Answer 3

It's not you. Whoever wrote that has never dealt with public facing web applications. As you have correctly noted, anyone with knowledge of this 'backdoor' can wreck havoc on your application.

Answer 4

It's the original developer being lazy when it came to both design and testing.

The ideal solution is separate out the validation or credit card authentication etc from the code into separate dlls/services/etc. The real versions can then be replaced by mocks to facilitate testing of the site. The services can also be tested independently.

These mocks never get anywhere near the production server so you should never have backdoors into your code.

You can also replace any/all of the services without having to update your application - as long as the new service presents the same interface as the original.

Answer 5

I'm going against the grain and saying querystrings make good debug switches.

Before everyone jumps on me - using querystring to disable validation is horrendously stupid, a giant security hole, and should never happen. You were completely valid and justified nuking the code immediately.

But for debugging, querystrings work great. We have a few querystrings that will turn on IDs for a few objects on our pages so we can quickly get them without logging into the production database (not everyone has access to the Prod DB of course), or for checking calculation components. You just have to be careful and intelligent about them.