As longtime readers know, I am also a software developer (we can’t call them engineers in Canada for legal reasons).
I took over a complex codebase last year in a field I’m unfamiliar with, and have been working on features and improvements since then. This week I decided that it was time to rename a bunch of objects because I didn’t like the previous naming convention as it was confusing.
Tonight while testing a new feature, I accidentally clicked on the wrong thing and the application crashed. I inspected the stack trace and discovered that a method was missing. This was of course impossible because I had used a well-known Visual Studio extension to refactor my code and after all, it compiled and ran.
Upon closer inspection, it turns out that there was what seemed like an innocuous piece of code that I’d dismissed previously because the same Visual Studio extension told me that it is never used. 0 references. It said it right there.
One of the things I changed during this refactoring process was taking twenty distinct objects (numbered 1..20) and put them in a generic
List<T>. What I had not counted on was that the “unused” code was actually called through .NET Reflection. Another method was building a string based on a database value, then invoking that as a method name in the code. Since I’d renamed a bunch of things, the code was failing because the method name was no longer there (it was one of the 20 objects I’d changed into a list). This code relies on what is effectively SQL injection.
It works like this: the database fetches a list of rows, and each row contains the name of a matching method in the code. It doesn’t take much effort to change that value in the database, because there’s literally a screen to do that in the app. The code performs a small string manipulation on that value to build a method name. For the sake of discussion, let’s call one example GetVehicles(). The database value is called “Vehicles” and this code slaps the word “Get” in front of that and … just runs it.
There’s nothing stopping a user from changing “Vehicles” to “Customer” or “User”. Yes the app will crash, but the great thing about .NET code is the stack trace. You can even dump out the memory if you want.
Please, if you’re writing code, don’t rely on
MethodInfo.Invoke using values from a database table without fully vetting the inputs. This is just bad news. Database parameterization won’t save you.
Share your SQL injection stories in the comments below.