CodeSOD: Filtering Out Mistakes

We all make simple mistakes. It's inevitable. "Pobody's nerfect," as they say, and we all have brain-farts, off days, and get caught up in a rush and make mistakes.

So we use tools to catch these mistakes. Whether it's automated testing or just checking what warnings the compiler spits out, we can have processes that catch our worst mistakes before they have any consequences.

Unless you're Jose's co-worker. This developer wasn't getting any warnings, they were getting compile errors. That didn't stop them from committing the code and pushing to a shared working branch. Fortunately, this didn't get much further down the pipeline, but said co-worker didn't really understand what the big deal was, and definitely didn't understand why there were errors in the first place.

In any case, here were the errors tossed out by the C# compiler:

Error: CS0165 - line 237 (593) - Use of unassigned local variable 'filter'Error: CS0165 - line 246 (602) - Use of unassigned local variable 'filter'Error: CS0165 - line 250 (606) - Use of unassigned local variable 'filter'Error: CS0165 - line 241 (597) - Use of unassigned local variable 'filter'

Now, let's see if you can spot the cause:

 if (partnumber !="") { string filter="(PartPlant.MinimumQty<>0 OR PartPlant.MaximumQty<>0 OR PartPlant.SafetyQty<>0)"; } else { string filter="PartPlant.PartNum = '" + partnumber + "'"; } if (plantvalue !="") { string filter= filter + ""; } else { string filter= filter + " AND PartPlant.Plant = '" + plantvalue + "'"; } if (TPlantcmb.Text !="") { string filter= filter + ""; } else { string filter= filter + " AND PartPlant.TransferPlant = '" + TPlantcmb.Text + "'"; }

C#, like a lot of C-flavored languages, scopes variable declarations to blocks. So each string filter... creates a new variable called filter.

Of course, the co-worker's bad understanding of variable scope in C# isn't the real WTF. The real WTF is that this is clearly constructing SQL code via string concatenation, so say hello to injection attacks.

I suppose mastering the art of writing code that compiles needs to come before writing code that doesn't have gaping security vulnerabilities. After all, code that can't run can't be exploited either.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!