Most SQL-injection articles set a horrible example for young programmers.
Here is a very typical “bad example” of why you need to escape user data before it goes into SQL queries:
(ed. The symbol « is a line break that’s not in the real code.)
The problem is: this is the wrong way to authenticate users. These examples are written for beginners to understand the importance of sanitizing input, but they also provide a model to those beginners for how user authentication works. And it’s a very bad model.
This is a long one, more after the break.
The only upside to authenticating this way is that you don’t expose any information on failure, that is, if I’m trying to hijack someone’s account, I can’t tell the difference between an invalid user name and a valid user name with a bad password. That’s good, but there are good reasons not to do this at the database level.
The “correct” way is not much more complex. Basically:
- Look up the record with the username only.
- Get the (hashed) password out of the database.
- Hash the submitted password.
- Compare the two hashes.
This is really not very hard to implement. In PHP:
There are two key differences between this method and the method so often espoused by tutorial writers:
- This method stores an encrypted password instead of plain text.
- This method differentiates between bad usernames and bad passwords.
1 should be obvious. Never store an unencrypted password. It’s extremely dangerous: if someone ever gets a look at the table, they can just read the users’ passwords—which may well be the same as their bank password (no it shouldn’t be, but it probably is). And it’s unnecessary. Every server-side language implements the MD5 hash, which is weak but works. Better options (like PHP’s crypt()) can use algorithms like Triple-DES, SHA1, Blowfish, or at least MD5 with a random salt.
But wait, #2, I said it was better not to distinguish between a bad username and a bad password, right? Well… yes, to the end user. In either case, I should display a message like “Bad username or password” to the person who tried to log in.
Internally, however, I want to know what happened. Is someone targetting known users, or just trying random combinations? How did they find real usernames? Where should I be improving security?
You’re also minimizing the number of user-submitted strings that get sent to the database. There are fewer opportunities for you to accidently allows an injection attack. If you have a policy on username syntax, you can keep yourself even safer by not talking to the database if the username is bad:
(I’ve omitted logging or real error-handling here. In a live version, I would probably wrap most of this in a
<a href="http://us2.php.net/manual/en/language.exceptions.php" onclick="window.open(this.href,'newwindow'); return false;">try</a> block, throw one of three types of exceptions, and do some logging in the
I’m going to end with a request: if you’re about to write a tutorial for beginners, please be aware of what you’re modeling in your examples. If you’re doing something you would never do, for the sake of simplicity or because it’s not the focus of the tutorial, point that out. Link to another tutorial or at least mention that it’s a bad way to do something.
Don’t send a quiet message that wrong is OK.