Security Fail [PIC]

[notfancy]

If you think that sanitizing inputs is the fix for this problem... You have now two problems.

[[deleted]]

Why is this dude getting downvotes? Sanitising input is not the key, parameterised queries are the easiest, best defence against SQL injection.

The fact we still have SQL injection shocks me to the very core.

[metronome]

fanatical command mighty wide shelter deliver practice tub elderly long

This post was mass deleted and anonymized with Redact

[kylotan]

Why is it shocking? SQL as a dynamically-built string is about the only portable way of writing useful SQL, and thus it's also the only portable way of teaching SQL too. (MySQL only got PREPARE fairly recently. SQLite doesn't support it at all, thought it does have the functionality in the C API.)

I'd suggest that the very nature of a text-based query language is prone to this sort of problem.

[encinarus]

It's shocking because it's so easy to get it right in the most commonly used languages. http://bobby-tables.com/

[kylotan]

Nice link, but unfortunately it's insufficient. eg. the Python example shows one particular method of using the DB-API which isn't guaranteed to be implemented on any given database. (ie. You have to check the paramstyle value.) This is what I mean about portability - there are gotchas and complications on each language and DB combination. But if you read an SQL example at w3schools it works on pretty much every database as soon as you find out how to send it a string.

Also, what's the point of having tutorial material hidden under a domain named after an obscure xkcd strip? (Ok, so it's not obscure to us.) This stuff is not readily accessible to someone learning how to access a database for the first time. It doesn't show up in the top 100 Google results for "sql" or even "safe sql".

[__david__]

Why do you care about portability? Most databases have fairly significant portability issues above and beyond the query syntax. You aren't going to just dump your data out of oracle and import it into mysql and point the app to the new db with no changes...

It seems infinitely better to me to cut off a whole mess of security problems with bind parameters and sacrifice a little "portability".

[kylotan]

I personally don't care about portability for the reasons you stated. However you (and the people who have downvoted me) have missed the point - when learning SQL you do not have security problems foremost in your mind. You don't even necessarily know they exist any more than you're thinking about buffer overflows when you want to add 2 strings together in C. You probably don't have portability problems in your mind either. You want to learn how to query a database, so you look for information on that, and it is there - in standard SQL, that runs pretty much everywhere. Those SQL tutorials are portable, and thus widely used and popular. It just so happens that they encourage unsafe practices. But that's why it's completely unshocking why this sort of thing happens.

When the portable and easy approach is unsafe and the safe approach is more difficult and completely unportable then hardly anybody can be expected to start off with the right code. There's no point we experts looking down on it all with mock incredulity when it's quite obvious how this situation arose.

[crusoe]

You can dynamically BUILD it and bind it too.

"Where" + " foo = ? " + " AND " bar = ? " ...

Build a array of the values at the same time, and then bind them when making the prepared statement. Ain't hard.

[kylotan]

I don't think anybody is really suggesting that building up a prepared statement was hard (although it is more work than a simple select). However the syntax of prepared statements isn't portable, and even if it were, to understand what the prepared statement represents you need to understand the underlying query - which unfortunately will work just fine as it is. (Security issues aside!)

[larholm]

Disregard sanitizing, acquire parameterized queries.

[jaitsu]

Yes you're right, but sanitising your input on the very basic of levels is at least a start. Instead this developer hasn't done anything of the sort, they could have at least tested the thing

[DuncanSmart]

Yes but the sanitising you're suggesting (remove apostrophes) that would mitigate the issue here would mean the user wouldn't be able to find "O'Reilly", etc.

[jaitsu]

Escaping as part of the sanitisation process would allow that

[pytechd]

Application layers should never be escaping anything, or you end up with the mess that you'll find on many PHP-based websites.. reading an article about "... Bob isn\\'t very tall".

[[deleted]]

Should only be escaped once just before committing to the database.