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!
It's horrifying whether it's common practice or not. +1 to you for nuking it with extreme prejudice.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With