Recently we had an issue on our site where someone attempted SQL injection via a cookie (we’ll call it lastID). NOC was in a frenzy and angry about how the cookie as an attack vector could be ignored. They had a developer create a class for managing cookies that will sanitize lastID (and eventually other cookies) to check that it is numeric.
However, I think that this is wrong. The problem wasn’t that lastID wasn’t numeric, it was that the query that uses it was vulnerable to injection. I’m in charge of the review of the code changes, and I want to bring up the fact that forcing lastID to be numeric is not particularly useful and perhaps even undesirable.
Is there a term or concept that describes sanitizing at the right time (i.e. sanitizing at query time rather than at cookie retrieval time), or “over-sanitizing” (e.g. forcing lastID to be numeric) that I can use to describe what’s happening/should happen in my review? The second part of Postel’s Law seems to apply here in my mind:
Be … liberal in what you accept
6
Your are right. Correct all your queries to injection-proof prepared statements, and stop worrying about the supplied data. Trying to sanitize inputs to be directly interpolated into SQL statements leads to web applications that reject last names containing apostrophes, or passwords containing certain special characters.
By the way, just because you are right doesn’t mean you will be able to convince NOC. Good luck with that.
3
I would simply point out that the vulnerability lies with the database procedure that can be exploited by SQL Injection. The cookie is merely an attack vector.
While it is important to take care of known attack vectors, the more comprehensive solution is to fix the vulnerability at the source. This will both solve the immediate issue of the cookie attack vector and other unknown (possibly future) attack vectors.
You’re definitely right. I think you’re actually hitting on two concepts.
-
KISS. (Keep It Simple) The basic tenet is that you use the simplest solution that actually solves the problem. (Making lastID numeric doesn’t really solve the problem, just that particular case)
-
DRY. (Don’t Repeat Yourself) If there was ever another place where this query was executed with some other input than lastID, you’d have to solve the problem of SQL injection there too. Just fix the query and it’s solved for all execution paths.
Moreover, the solution of enforcing lastID to be numeric is using the wrong level of abstraction.
2