14 comments

[ 3.3 ms ] story [ 46.6 ms ] thread
I am curious, why would you bother with all the regex filtering instead of proper escaping or (even better) using prepared statements?
I think the main point is - don't do that, because you can go around regex rewriting. It was an exercise in hacking the regex protection, not in protecting via regex.
I think the point is that it's possible to get pass poor programming and filtering. A smart developer would've been aware of intval() and used it instead of mysql_real_escape_string().

I certainly worked with my share of developers with just enough brains to know they need to validate the input and might have done their own regex method to filter instead of relying on far simpler built-in methods. Prepared statements being the next step up.

and if you were determined to use a regex, wouldn't you create a whitelist pattern (i.e. just numbers) rather than a blacklist (no quotes, whitespace, etc)?
Yes. Better yet: if your parameter is numeric (say, a LIMIT), typecast it to an integer (to_i, int(), etc).
After some thought, the number one reason you'll see this is the following sequence: Initially, you didn't know anything about prepared statements or encoding properly. (The vast majority of programmers do not actually understand encoding; even quite a lot of people parroting "Use prepared statements!" are parroting the right thing without really, deeply understanding it.) Then somebody accidentally enters an apostrophe or something and your code breaks. You are a novice developer, so your first inclination is to strip out the apostrophe. Repeat for a few more characters. Arrive at a point where you can no longer think of a way to exploit your code, assume that therefore nobody can, call it a day.

I had some other reasons here, but that is almost certainly the dominant reason; it explains all the instances of it I've encountered.

(Understanding encoding: http://www.jerf.org/iri/post/2548 )

Edit: BTW, when I say a lot of people parrot "use prepared statements" without understanding, please note I am not accusing any other commenter in this thread. I am actually thinking of people I've encountered in real life who A: told me that but B: I have other reason to believe don't really understand why. In particular, I work a lot professionally in Perl, and the Perl culture has somehow managed to converge from top to bottom on prepared statements, and I will confidently be told to use prepared statements by people who will turn around and write the most trivial of XSS attacks by the bushel. (If you don't know the connection between those two things, see my link above.)

$id = (int)$_GET['id'];
The data validation is so you can display a nice error message. To protect the database, you need to use prepared statements instead of mysql_really_really_try_hard_to_quote_the_string_or_something.
In the particular case of integer ids, however, can't you just make sure what's being passed is an integer? Similarly, for a 'simple' username, check against "^[a-zA-Z0-9]+$"?
Casting the ID to an int (like you should do) will make any non-numeric strings into 0. If your ID is 0 you can assume an attack and give such a message. With PHP you should be using mysqli_ functions instead of mysql_ variants as they protect against multiple queries executing in one mysqli_query() call.

I do agree prepared statements are the way to go at least 90% of the time.

Out of curiosity, do these attack vectors still apply to MySQL prepared statements? From his query example, the prepped version would read "SELECT id,name,pass FROM users WHERE id = ? AND pass = ?" and the two parameters would be passed in as an array of strings.
> do these attack vectors still apply to MySQL prepared statements?

No.

Sure they do. All you have to do is find a query that uses user input to select a table, or that accepts an offset for pagination and doesn't explicitly typecast it. We find SQLI in parameterized queries all the time.

You should absolutely use prepared statements, but don't kid yourself about their magic powers.

By the time you're trying to figure out how to keep an SQL Injection vector from becoming exploitable, you've already lost.

Just use prepared statements, and make sure that anything that isn't parameterizable (table names, column sort orders, limits, offsets, etc) is hardcoded and/or typecast (for instance, keep a table of valid table names, and look them up from HTTP parameters).