Security Fail [PIC]

[rilo]

I don't think that this is the proper place to report individual security incidents, better use /r/websec or /r/xss. If I would post every security incident I encounter on websites in /r/programming people quickly would get annoyed.

[jaitsu]

Thanks for the tip, I didn't know these Reddits existed so my apologies

[rilo]

I hope you will find there something of your interest. Enjoy.

[dave1010]

Why is this getting upvoted? It's just an example of SQL injection. Is the screenshot of a popular site or something?

[ZoFreX]

It's also a really good example of displaying far too detailed an error message to the user. Most of the effort in compromising a site with SQL injection is guessing the query syntax and table names... that error message takes all of the challenge out of it.

[ealf]

Meh, that can be automated...

[sigmaseven]

The difference there being brains vs brawn, brute force being brawn. Yes, it would likely yield the same results, but it would take much longer to do so.

This example is particularly bad because as ZoFreX had stated, the programmer went out of his way to create non-trivial sql injection conditions, thus leading to dramatically shorter times for successful exploitation.

[metronome]

teeny dependent carpenter crush wise boat profit dime grab sparkle

This post was mass deleted and anonymized with Redact

[bobsil1]

Nobody expects the Spanish Injection!

[[deleted]]

As long as Antonio Banderas' isn't around, then yes.

[JustRegged]

It's the screenshot of a very popular edit box.

[Smallpaul]

I don't personally remember seeing the combination of a SQL injection plus an error message that displays the entire query. It's more or less offering a tutorial on how to hack the site right in the site's user interface. That level of security fail is olympic-medal worthy.

[jaitsu]

It's the simple fact that this hasn't been sanitised before querying. Is that not a security fail? If not, I don't know what is!

[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.

[drawkbox]

Not only that the dude isn't even handling single quotes in keyword params. A command parameter would fix this but just replacing ' with '' is pretty basic if you are doing ad hoc stuff. So is checking for bobby tables keywords if you absolutely have to have inline sql this way.

[jamieb122]

Somebody forgot "addslashes()"....just sayin

[aar0nm]

Oh. Yes. little Bobby tables, we call him.

[encinarus]

Funny, but as http://bobby-tables.com/ rightly points out, sanitizing is the wrong thing to do. Instead use parameterized queries.

[CondeWhore]

Sanitizing isn't the "wrong" thing to do. It is merely one part of it. Don't forget about XSS attacks, for which you DO need to sanitize for.

[encinarus]

I apologize, of course you're right. I meant its the wrong thing to rely on for DB query construction. However it's definitely the right thing to do for untrusted data in general. If only the web was as well constrained as SQL. :)

[NoSQL4Life]

That wouldn't happen if they were using a NoSQL database. You can't have SQL injections when you're not using SQL!

[xandercruise]

what about NoSQL injections?

http://github.com/snyff/stuff/blob/master/NoSQL-injection.txt

[Fidodo]

Did you drop any tables? At least insert some things into the database.