The Daily WTF: Curious Perversions in Information Technology

dkf · 2012-02-09 Reply Admin

Nebs:
In order of how problematic different approaches are, I would rate the types as follows:

(very bad) Handwritten SQL inside PHP

(bad) Handwritten SQL with mysql-real-escape-string()

(bad) Handwritten SQL inside stored procedures

(excellent) Automatically generated SQL made by a framework

You should also learn about prepared statements. Basically, the SQL code becomes a template that is substituted into at execution time, with this substitution being done by the database engine rather than your code. It gives very similar speed to stored procedures combined with the flexibility of hand-written SQL (frameworks often only support a small subset of the queries that your app needs). The advantage over escaping is that you don't need to remember to get the escaping done right when you're using prepared statements. (It's even nicer if you have named substitution variables, but that's not universally supported.)

2012-02-09 Reply Admin

Jay:
big picture thinker:
Tim:
you don't even need to use prepared statements; you can just write a function called QuoteStringForDatabase and use this instead of putting quote characters round the value
Actually, you really do need to use prepared statements.
Sanitizing inputs is so '00s. Lets learn new things.

Let's assume that Tim's QuoteStringForDatabase properly escapes single quotes, and all other magic characters that the db engine recognizes, if any.

Under what circumstances would this function not work but prepared statements would?

The problem is Global Cooling. Rather than asking an SQL server to efficiently search for and return the one piece of data you need; it's far better to ask the server for a huge list of data and then use an inefficient scripting language to plough your way through that huge list. This helps to keep million of miles of network cabling and a milliad of computers generating heat, to prevent the effects of Global Cooling.

Note: Global Warming is a just an indicator that the "prepared statements" scheme is successfully preventing Global Cooling.

2012-02-09 Reply Admin

SEMI-HYBRID code:
...actually, every time i see something like this, i wonder if i'm doing it wrong, or just people don't get how simple it can be... i run every user input through htmlspecialchars($blah, ENT_QUOTES), and i NEVER got any SQL injection by using this (i tried to attack one of my sites using this by all SQLI methods i know of, none worked).

Yes you're doing it wrong. For starters, htmlspecialchars is meant for encoding HTML, not SQL - so if it has any effect whatsoever it's not because it was designed to.

A classic one I encountered last week was somebody converted ' to ''. Great, no injection you say? Until you realise the code later on trims that string down to 8 characters, so if you type aaaaaaa' then you end up with aaaaaaa'' which then gets cut back down to aaaaaaa' and the next input parameter is free to execute anything.

To quote Schneier:

'Anyone can invent a security system that he himself cannot break. I've said this so often that Cory Doctorow has named it "Schneier's Law": When someone hands you a security system and says, "I believe this is secure," the first thing you have to ask is, "Who the hell are you?" Show me what you've broken to demonstrate that your assertion of the system's security means something.'

Everyone thinks the system they write is secure because the protections they put it place are designed to protect against an attacker with an identical skillset. In the real world, your attacker is never a mirror image of yourself.

Hackers prey on assumptions. Systems change over their lifecycle and few people actually read the specifications for the systems they use. Always assume the attacker is a much more skilled person than you (if it helps, imagine they are an autistic child in their basement with OCD who has no life - whatever fantasy scenario drills the idea in).

2012-02-09 Reply Admin

SEMI-HYBRID code:
i run every user input through htmlspecialchars($blah, ENT_QUOTES), and i NEVER got any SQL injection by using this (i tried to attack one of my sites using this by all SQLI methods i know of, none worked).

And you know more SQLI methods than all the hackers in the world, and their scripts. But you still don't know the difference between htmlspecialchars and parameterized queries!

Hint:

Parameterized queries are used by professionals.

htmlspecialchars is used (for fake SQLI protection) by stupid people and trolls.

no laughing matter · 2012-02-09 Reply Admin

FragFrog:
And input validation is not just limited to webdevelopment. I daresay it is even more important when working on system kernels, and it is downright essential for that man working on radiotherapy machines: just imagine (long story ...)

Why limit yourself to imagination?

Welcome to reality!

In three cases, the injured patients later died from radiation poisoning.

2012-02-09 Reply Admin

Ah, the days that a DailyWTF could be remotely interesting. Not anymore.

no laughing matter · 2012-02-09 Reply Admin

SEMI-HYBRID code:
...actually, every time i see something like this, i wonder if i'm doing it wrong, or just people don't get how simple it can be... i run every user input through htmlspecialchars($blah, ENT_QUOTES), and i NEVER got any SQL injection by using this (i tried to attack one of my sites using this by all SQLI methods i know of, none worked).

Yes, i really wanted a list of all students whose age is

20; DROP TABLE STUDENTS; --

2012-02-09 Reply Admin

Indeed it's not. mysql_no_seriously_we_got_it_right_this_time() is the way to go

2012-02-09 Reply Admin

TRWTF is that the "Location" header used to redirect to index.html is lowercased

2012-02-09 Reply Admin

"Why, a four year old child could understand this report!" (turns to his secretary): "Run out and find me a four year old child, I cannot make head or tail of it." -- Groucho Marx

2012-02-09 Reply Admin

Nagesh:
I am waiting to see some guy bring out Boby Tables
[image]

Why is that guy hooked up to a Super Nintendo?

2012-02-09 Reply Admin

Sea Sharp:
(...)I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.(...)

And I've written a couple thousand lines of .NET and Java, yet I more often than not don't understand some of the .NET or Java WTFs on here. Not everybody's a genius master programmer of APLEC (All Programming Languages Ever Conceived), so please keep us lowly, non-all knowing developers in mind before blinding us with your all-illuminating greatness.

Captcha: Ideo. Thanks for making us feel like ideots

Anketam · 2012-02-09 Reply Admin

The poop... of DOOM!:
Sea Sharp:
(...)I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.(...)
And I've written a couple thousand lines of .NET and Java, yet I more often than not don't understand some of the .NET or Java WTFs on here. Not everybody's a genius master programmer of APLEC (All Programming Languages Ever Conceived), so please keep us lowly, non-all knowing developers in mind before blinding us with your all-illuminating greatness.
Captcha: Ideo. Thanks for making us feel like ideots

Anyone can easily write bad code, but it takes a brilant developer to figure out what the hell it actually is doing and in theory what the developer was actually trying to do.

2012-02-09 Reply Admin

Some Damn Yank:
Guest:
Sure use Regex, much better. Hope that was meant as Joke... Also mysql_real_escape_string is not safe at all by itself.
New here? A favorite pastime at the DWTF is to find a more elegant way to express the original WTF without fixing the root problem. Such as using a regex in place of a long if/elseif/elseif/elseif... mess - but without fixing the root problem. It's one of the reasons I like this site :-)
Captcha: appellatio. An obscene act performed on an apple.

Ok, let's give this a go. This way, you don't even need to check the input!

$sql = "select muguser_id, muguser_directory " . 
       "from mugusers " . 
       "where muguser_active = 1 " . 
       " and muguser_email = '" . $_POST["email"] . "' ";

$result = mysql_query($sql);
if(mysql_insert_id() > 0 ) { // Check for injection
  print("SQL INJECTION DETECTED! YOU DIRTY HACKER, YOU!");
}
else {
  // Shenanigans
}

2012-02-09 Reply Admin

C-Octothorpe:
Jerry:
C-Octothorpe:
your attitude is a prime reason why the developer who wrote this likely didn't ask anybody if there was a better way to protect against SQLi.
//prevent sql injection
So you're defending a "developer" who wrote this and knew enough to arrive at the magic words "SQL Injection" but had never heard of Wikipedia?
http://en.wikipedia.org/wiki/SQL_Injection
Saying "herp derp, why didn't they check wikipedia" is oversimplifying the problem to the point of being laughable. It could have been anything, really. Maybe he saw all the "senior" devs and coworkers do this so why question it. Or maybe the code was written long before SQL injection was as popular among the developer community as it is today.

Lack of knowledge is always the root cause which can happen because lack of training, best practice like code reviews or using code analysis tools, etc.

I'm not defending him, I'm just against that attitude which is a reason (not the only reason) why some developers do shit like this.

Back when I was learning PHP, before the days of Wikipedia, I heard about SQL injection too. I ended up with similar code as the OP (although still more elegantly done, and more effective, mostly checking for all kinds of variations on ', like ´ and `). I was still learning and didn't have any idea of what prepared statements were. That on itself isn't a bad thing. Everybody's got to learn, and every self-taught PHP dev. made that mistake at one point or another.

The actual WTF is it being in production. If it's not a brother-in-law's cousin's friend's neighbour's kid who'll make a site for a popsickle and a balloon kind of thing, it's a full-blown WTF. Otherwise... meh, just learning process.

2012-02-09 Reply Admin

Sea Sharp:
TheSHEEEP:
Okay, for the non-PHP, non-SQL people among us... could someone please explain what's going on here?
Now, hopefully this can be transmitted without sounding like a troll (ahem), but I do have an honest question about this question:
When someone says, on hereyou don't even need to use prepared statements; you can just write a function called QuoteStringForDatabase and use this instead of putting quote characters round the value , that they aren't familiar with <language> and would like someone who is to explain, what are they actually saying? I guess I'm asking coming from the position that there aren't many languages (at least within a familiar paradigm) that can just be read by anyone who understands programming. I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.

CAPTCHA: venio "venio, vedio, viccio" -> "I come because of the vice Vedius had." (Damn that Vedius.)

If he hasn't used SQL, though, the idea of SQL injection attacks could potentially be quite foreign. I mean, in what other situation do you drop user input directly into executing code?

For the original poster of this question, assuming zero knowledge of SQL and very little of PHP: Basically, what they're doing is they're crafting a string (presumably, after this code) that is SQL code, and some of the items within that string will be the variables input by the user. So they might have, for example, "SELECT * FROM users WHERE username = $username" where $username is going to be replaced by whatever the user input. Note that because of the way SQL and PHP interact, it's going to be replaced long before it gets to be executed by the SQL server -- the server just sees a literal string. So it's KINDA like a buffer overflow, in that you can, say, insert a semicolon to end the statement and then insert a new statement of your own (say, delete all data) into the username field and the server will just go on and happily execute that. Or you could do something like 'sometext OR 1' and the server will then select the every user from the list instead of whatever you put in (the OR 1 being 'true') So they're trying to check for that sort of thing, but going about it in the worst and most ineffective way possible.

2012-02-09 Reply Admin

The poop... of DOOM!:
C-Octothorpe:
Jerry:
C-Octothorpe:
your attitude is a prime reason why the developer who wrote this likely didn't ask anybody if there was a better way to protect against SQLi.
//prevent sql injection
So you're defending a "developer" who wrote this and knew enough to arrive at the magic words "SQL Injection" but had never heard of Wikipedia?
http://en.wikipedia.org/wiki/SQL_Injection
Saying "herp derp, why didn't they check wikipedia" is oversimplifying the problem to the point of being laughable. It could have been anything, really. Maybe he saw all the "senior" devs and coworkers do this so why question it. Or maybe the code was written long before SQL injection was as popular among the developer community as it is today.

Lack of knowledge is always the root cause which can happen because lack of training, best practice like code reviews or using code analysis tools, etc.

I'm not defending him, I'm just against that attitude which is a reason (not the only reason) why some developers do shit like this.
Back when I was learning PHP, before the days of Wikipedia, I heard about SQL injection too. I ended up with similar code as the OP (although still more elegantly done, and more effective, mostly checking for all kinds of variations on ', like ´ and `). I was still learning and didn't have any idea of what prepared statements were. That on itself isn't a bad thing. Everybody's got to learn, and every self-taught PHP dev. made that mistake at one point or another.

The actual WTF is it being in production. If it's not a brother-in-law's cousin's friend's neighbour's kid who'll make a site for a popsickle and a balloon kind of thing, it's a full-blown WTF. Otherwise... meh, just learning process.

Some people know what prep. statements are and will still tell you that you're a damn noob for using them. like me.

http://blog.ulf-wendel.de/?p=187

it's totally possible to clean your input to require no prep. stmt or stored proc.
stored procs can be much better than prepared statements, especially when you get a lot of reuse (they're global resources, not limited to a conn.)
prepared statements are the noob webdev's shortcut to sqlInjection, but that's not what they're good at. The main positive point about prepstmt is killing the query planning overhead for repeated queries.
When you don't know why it's great, don't go sayin' everyone should do it and it's the best solution
you damn fools.

2012-02-09 Reply Admin

If there by any chance is anyone out there that haven't yet heard of little Bobby Tables - here is the exploit of a mom: http://xkcd.com/327/

CAPTCHA: suscipit That is some suscipit looking code man, is that SQL really injected properly?

Coyne · 2012-02-09 Reply Admin

The poop... of DOOM!:
Some Damn Yank:
Guest:
Sure use Regex, much better. Hope that was meant as Joke... Also mysql_real_escape_string is not safe at all by itself.
New here? A favorite pastime at the DWTF is to find a more elegant way to express the original WTF without fixing the root problem. Such as using a regex in place of a long if/elseif/elseif/elseif... mess - but without fixing the root problem. It's one of the reasons I like this site :-)
Captcha: appellatio. An obscene act performed on an apple.
Ok, let's give this a go. This way, you don't even need to check the input!
$sql = "select muguser_id, muguser_directory " . 
       "from mugusers " . 
       "where muguser_active = 1 " . 
       " and muguser_email = '" . $_POST["email"] . "' ";

$result = mysql_query($sql);
if(mysql_insert_id() > 0 ) { // Check for injection
  print("SQL INJECTION DETECTED! YOU DIRTY HACKER, YOU!");
}
else {
  // Shenanigans
}

...and let us count the WTF's:

Basic design allows SQL injection.
The fix only detects inserts, so if the injection attack is a DELETE TABLE X, well, too bad.
The fix only detects the insert injection after the insert was done.("Well, yes, all the horses did escape before we closed the barn door. But at least it's closed.")
Only detects the insert injection if it was the last statement the hacker included in his attack.
Provides a nice confirmation message to the hacker so he knows he's on the right track.

All-in-all, a really good example for the WTF mill. :)

2012-02-09 Reply Admin

Matt Westwood:
Nebs:

Seriously, do yourself a favour and check out some different frameworks. I use CakePHP but there are loads of other ones out there. They will take your programming up a couple of levels, in way less time than you might think.

Stupid cunting troll who needs his nuts kicked out of his gob. If you come anywhere near any of the programs I'm responsible for I'll fucking kill you, you fucking shithead.

I love this. You can see the learning curve for autogenerated sql across these 2 posts.

Learn sql
Discover frameworks that will automatically generate your sql for you yay shiny toys that cut my development time to nothing
5 years go by in which you've had to upgrade and maintain systems that used those frameworks. You come across situations that actually involve some complex sql that you can't just generate from the framework and you need to twist and contort your program/self to get it to work. You actually run a trace on the database and see what the lousy framework is generating and find you can run those same statements 3x as fast if you'd just stayed at step 1. You try other frameworks across other languages and find they all have the same problems.
End up jaded and possibly perpetually enraged. The world seems a darker place. The mere mention of autogenerated sql makes your eye twitch and you hands flex.
Return to step 1. You may look at new frameworks to see if they got it right this time, but probably not. You would only find they are even worse than before. You've sought counseling and can return to society (assuming they didn't catch you when you offed the guy that introduced you to frameworks.) You try to save others the pain of letting frameworks do their job for them.

2012-02-09 Reply Admin

I certainly hope this is some kind of troll joke. Selecting the entire table to avoid injections instead of just simply using prepared statements. You should be flayed for suggesting it.

CAPTCHA: gravis. This is a gravis travesty

olddog:

// The first WTF is the use of double-quotes ...
// so lets use single quotes so php doesn't evaluate the post vars
$u = trim(stripslashes($_POST['username']));
$p = trim(stripslashes($_POST['password']));
$e = trim(stripslashes($_POST['email']));
// filter padded spaces and escaped chars .. that's easy enough
if(strlen($u.$p.$e) != strlen($_POST['username'].$_POST['password'].$_POST['email'])
{
bail(); // some bailout function that exits gracefully
}
//at this point we really don't care if the user name is something like "Egor or Orig";
// if the mugusers table isn't too large...deny injection by reverse comparison
$rsrc = mysql_query('SELECT * FROM mugusers WHERE mugusers.active = 1');
$okay = FALSE;
while($row = mysql_fetch_object($rsrc))
{
if(($row->username) && $row->username == $u)
{  if(($row->password) && $row->password == md5($p)// assuming md5 encryption
{  if(($row->email) && $row->email == $e)
{

$okay = TRUE; break;
}
}
}
}
if(!$okay)
{
bail();
}
//---- it's okay from this point

2012-02-09 Reply Admin

Ninja:
As for your previous question, input cleansing is not a completely reliable method and there are ways that people can still bypass those methods.

Okay. Please give me one example. I'm not trying to be snide, maybe there is such an example and I am not aware of it. If there is, I'd be interested to hear about it.

Ninja:
The flipside of your question is what is the harm in having prepared statements? ...

I have nothing against prepared statements. I use them all the time. But I also use escaping functions all the time. It's nice to have more than one tool in the toolbox, so you can use each when it's more convenient.

2012-02-09 Reply Admin

Jerry:
1. White lists FTW. List what you know is good. Reject the rest. 2. Black lists WTF. Don't do that.

A white list is rather impractical in this case. The white list is the entire unicode character set, with the exception of single quotes, sometimes the backslash, and maybe some db engines have another character or two. Am I really going to make a white list of 2^64-2 valid characters, rather than blacklist the two?

Jerry:
3. Even if you know all other magic characters that the db engine recognizes (and you don't) the list will change tomorrow.

Of course I know what they are, or at least I can easily find out. They should be documented. For example, MySQL documents theirs here: http://dev.mysql.com/doc/refman/5.6/en/string-literals.html. They had better be documented, because if you cannot readily find what the magic characters are for your db engine, how could you use the magic characters? And you could never be sure how it would interpret any value you gave it. Writing any string would be a guessing game.

They better not change tomorrow without ample warning, or any SQL statement that uses constants could break. Surely there is plenty of SQL out there with hard-coded constant values, like "select ... where foobar like '$%.__'" If tomorrow a db vendor decided to make '$' a magic character, all sorts of SQL would break.

This objection is like saying that you shouldn't use "where" clauses because you don't know the syntax and it might change tomorrow.

So assuming I go to the elementary effort to read the documentation to find out if my db recognizes any magic characters besides a single quote and then properly escape them -- which I've done the few times I wanted to write an escape function, it only takes a few minutes if the documentation is remotely well organized -- is there any case where this would not work but using a prepared statement would work?

2012-02-09 Reply Admin

Brendan:
Jay:
big picture thinker:
Tim:
you don't even need to use prepared statements; you can just write a function called QuoteStringForDatabase and use this instead of putting quote characters round the value
Actually, you really do need to use prepared statements.
Sanitizing inputs is so '00s. Lets learn new things.

Let's assume that Tim's QuoteStringForDatabase properly escapes single quotes, and all other magic characters that the db engine recognizes, if any.

Under what circumstances would this function not work but prepared statements would?

The problem is Global Cooling. Rather than asking an SQL server to efficiently search for and return the one piece of data you need; it's far better to ask the server for a huge list of data and then use an inefficient scripting language to plough your way through that huge list. This helps to keep million of miles of network cabling and a milliad of computers generating heat, to prevent the effects of Global Cooling.

Note: Global Warming is a just an indicator that the "prepared statements" scheme is successfully preventing Global Cooling.

I think you're addressing a different question. I wasn't suggesting implementing the WHERE clause on the client.

2012-02-09 Reply Admin

SELECT * FROM CUSTOMER.TRANSACTIONS WHERE ID = _____;

For _____ the hacker sends: 123 OR 4 < 5

No quote, no backslash, no semicolon... must be OK, right?

2012-02-09 Reply Admin

Danny 'Rushyo' Moules:
A classic one I encountered last week was somebody converted ' to ''. Great, no injection you say? Until you realise the code later on trims that string down to 8 characters, so if you type aaaaaaa' then you end up with aaaaaaa'' which then gets cut back down to aaaaaaa' and the next input parameter is free to execute anything.

Umm, yeah, if you screw up doing the escape, your program will fail. That's not an argument against doing escapes: that's an argument against having bugs in your code.

And sure, I've seen dumb mistakes like that. I once worked on a program that did HTML escaping, and THEN truncated the text to a maximum length. Then they hit a case where there was an ampersand in the last few characters, it got translated to "&", and then they chopped off the "p;" and it displayed on the screen as "&am", which looked weird to the user. The easy solution is to do any truncation BEFORE doing escaping.

Danny 'Rushyo' Moules:
'Anyone can invent a security system that he himself cannot break. ...

Okay, there's truth in that. But there's also truth in reading and understanding the specs and then designing your system accordingly. It's a mistake to over-simplify the complex, but it's also a mistake to over-complicate the simple.

Safely escaping a SQL string means looking for TWO special characters and escaping them. It's not that hard. I suppose it's easier to use prepared statements or parameterized queries, because then you may not even need to look at the specs. You don't have to worry about possible differences between the DBMS you're using today and the one you used at your last job. Etc. (That's why I'm using parameterized queries on my current project -- there was no reason not to and it's easier.) But if someone has a reason to write an escape function, it's not that big a deal. Some of the posters here make it sound like it's some hugely complex task that no mortal programmer could hope to get right.

Matt Westwood · 2012-02-09 Reply Admin

Anketam:
The poop... of DOOM!:
Sea Sharp:
(...)I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.(...)
And I've written a couple thousand lines of .NET and Java, yet I more often than not don't understand some of the .NET or Java WTFs on here. Not everybody's a genius master programmer of APLEC (All Programming Languages Ever Conceived), so please keep us lowly, non-all knowing developers in mind before blinding us with your all-illuminating greatness.
Captcha: Ideo. Thanks for making us feel like ideots
Anyone can easily write bad code, but it takes a brilant developer to figure out what the hell it actually is doing and in theory what the developer was actually trying to do.

Not necessarily - just a different skillset. I've met developers who can rustle up a phenomenally intricate and elegant full-blown application, but who can't debug a 20-line method. I've seen ineffectual code monkeys who have proven themselves to be incompetent at writing apps to be absolutely mustard when it comes to fixing stuff that's broken.

The enlightened manager moves people to where they are most effective at doing what they're good at. A less well enlightened manager (also known as "a cunting arsebrained fuckwitted pointy-haired COBOL-reject") insists on giving people stuff to work on which they are rubbish at (and in fact don't really like doing), on the grounds that "practice makes perfect" and "I want a team of all-rounders" etc.

C-Octothorpe · 2012-02-09 Reply Admin

FragFrog:
is it really too much to ask that a developer knows at least the basics of his trade?
[snip]...

I too made this mistake, many years ago. I learned from that, but not before thousands of users lost their data.

Is it just me, or are you completely contradicting yourself here? How can you feel completely entitled to blast someone for a common fuckup (to the point of suggesting some sort of regulation), yet you go on to say "hey, I did that too!"?

FragFrog:
I know for a fact that in some countries, you cannot sign off on an building drawing before you have at least several years experience as a junior engineer. A doctor usually spends years looking over the shoulder of a more experienced physician. Even if you want to drive a forklift, you need to have a certification before you are allowed anywhere near that thing. For almost any profession you care to name, a long track of apprenticeship and learning is considered normal. But when it comes to websites, apparently the attitude is "well, he should just be able to figure it out on his own and everyone should be kind and gentle when a mistake is made".

What hoodaticus said... Besides, the industry self-regulates to a point. The problem is that shitty developers will still find places of employent when really they should be getting turned down at the phone screening phase. This would encourage/force them to learn-up, which is what "good" developers do on their own accord.

FragFrog:
And input validation is not just limited to webdevelopment.

I didn't say that at all, but using a blanket statement like "all programmers should know SQL injection and how to defend against it" is just misguided.

FragFrog:
If you really thing that proper input validation is limited to webdevelopment, I fear you are the ignorant one here.

u mad bro?

FragFrog:
Bottomline: some principals are so fundamental that there is simply no excuse for not learning them. Sometimes that means listening to the experienced guys to tell you how it works and what to do; no, nobody likes admitting ignorance, but a bruised ego heals much easier than ten million leaked credit-cards.

If there was no excuse, then sites like this wouldn't exist. The big assumption is that all experienced guys know themselves how to do it correctly, and many do not. It has very little to do with bruised egos and more to do with education and guidance. Like you said yourself, you made stupid programming mistakes however you were swiftly guided by knowing hands on how to do things correctly.

2012-02-09 Reply Admin

Ok so I think you're really missing the point here (partially because I didn't phrase my response very well).

Hypothetically speaking if you do indeed create a function that perfectly escapes the input strings before executing the SQL query, then chances are that you are fine.

However, there are times when your escape function may not turn out to be as exhaustive as you thought. You may miss some special characters, or overlook some special cases. These exceptions will likely be dependent on how you implement your escaping function. Assuming that your code is perfect at any time is an awful mistake to be making in our trade. This is what testing is for, but safety nets do sometimes fail.

Furthermore, as other people have discussed, there may be scenarios in which your special characters will become outdated due to changes to the DB system you are using. In this case, you will have to update your escaping function or risk being screwed over due to changes that were not entirely in your control. Even though you expect them to be "announced well in advance," assuming that they will be is probably a mistake. Maybe you have more faith in others than I do, but I'm a pretty cynical person and I don't trust other people to notify me of important things.

So really, sure, you could hypothetically write a "perfect" escaping function and have a decent chance of being safe. But it's kind of like riding a motorcycle: You may be an excellent rider, but you still should wear your helmet just in case.

I hope this answers your question a little better.

2012-02-09 Reply Admin

Because the Human Interface Device they have there looks really unsuitable for getting a clean ECG trace. Actually it looks like several PlayStation Move wands.

2012-02-09 Reply Admin

some EMT:
Because the Human Interface Device they have there looks really unsuitable for getting a clean ECG trace. Actually it looks like several PlayStation Move wands.

Does the reply create any sort of connection to the post it came from? I was referring to the second posting of the doctor picture.

PedanticCurmudgeon · 2012-02-09 Reply Admin

some EMT:
some EMT:
Because the Human Interface Device they have there looks really unsuitable for getting a clean ECG trace. Actually it looks like several PlayStation Move wands.

Does the reply create any sort of connection to the post it came from? I was referring to the second posting of the doctor picture.

Congrats on discovering the quote button. Now, can you tell us what it does?

2012-02-09 Reply Admin

Danny 'Rushyo' Moules:
SEMI-HYBRID code:
...actually, every time i see something like this, i wonder if i'm doing it wrong, or just people don't get how simple it can be... i run every user input through htmlspecialchars($blah, ENT_QUOTES), and i NEVER got any SQL injection by using this (i tried to attack one of my sites using this by all SQLI methods i know of, none worked).

Yes you're doing it wrong. For starters, htmlspecialchars is meant for encoding HTML, not SQL - so if it has any effect whatsoever it's not because it was designed to.

A classic one I encountered last week was somebody converted ' to ''. Great, no injection you say? Until you realise the code later on trims that string down to 8 characters, so if you type aaaaaaa' then you end up with aaaaaaa'' which then gets cut back down to aaaaaaa' and the next input parameter is free to execute anything.

To quote Schneier:

'Anyone can invent a security system that he himself cannot break. I've said this so often that Cory Doctorow has named it "Schneier's Law": When someone hands you a security system and says, "I believe this is secure," the first thing you have to ask is, "Who the hell are you?" Show me what you've broken to demonstrate that your assertion of the system's security means something.'

Everyone thinks the system they write is secure because the protections they put it place are designed to protect against an attacker with an identical skillset. In the real world, your attacker is never a mirror image of yourself.

Hackers prey on assumptions. Systems change over their lifecycle and few people actually read the specifications for the systems they use. Always assume the attacker is a much more skilled person than you (if it helps, imagine they are an autistic child in their basement with OCD who has no life - whatever fantasy scenario drills the idea in).

Yes, yes and yes.

We had an argument with Security people who thought we should write a Threat Risk Assessment for our application. We pointed out that:

We are not security experts and don't necessarily understand where threats are coming from and how
We are developers. Developers are arrogant. I present code that is perfect, and could not possibly see vulnerabilities in my own code (other than artifical ones)
We really don't like writing documents

We lost the argument, so we copied a different TRA replacing application names and where necessary Data Centre locations. We also snuck in most of the words to "Come mr tallyman, tally me banana" throughout the document, and (frighteningly) this was never picked up by the security team who was so keen to review our TRA...sigh

On a vaguely related note, this is also (as I understand it) one of the flaws in Six Sigma. One of the measurables is bugs vs potential bugs. The problem is that generally, if you know about a bug, you try to fix it, whereas quite often you are unaware a bug even exists. So a system might have 1 "existing bug" and 100 "potential bugs" (READ: "bugs we have fixed/avoided"). The problem is that a potential buyg is essentially a known fixed bug and an existing bug is a known bug. There are still potentially many, many as yet unknown bugs that we haven't taken into account.

Consider the following teller machine I was writing:

int withdrawCash(int requestedAmount)
{
  // we all know the cool cats deal in cents
  return 5000;
}

Jo (the new grad) tested this, and each time he tried withdrawing $50 he got it. He soon realised, however, that his bank balance wasn't changing. This becomes a known bug, so we have 1 known bug and (by our assessment) 1 potential bug (a known bug must also be a potential bug, right?). So we fix it:

int withdrawCash(int requestedAmount)
{
  //Defect 762:  Balance not updating;
  currentBalance -=5000;

  // we all know the cool cats deal in cents
  return 5000;
}

This time, Jo is happy and we ship to production this robust code which has no bugs and 1 potential bug. Then someone decides they want to take $20 out.

2012-02-09 Reply Admin

A whole lot of people commenting on this post seem to have a complete inability to grasp sarcasm.

2012-02-09 Reply Admin

Ninja:
Ok so I think you're really missing the point here (partially because I didn't phrase my response very well).
Hypothetically speaking if you do indeed create a function that perfectly escapes the input strings before executing the SQL query, then chances are that you are fine.

However, there are times when your escape function may not turn out to be as exhaustive as you thought. You may miss some special characters, or overlook some special cases. These exceptions will likely be dependent on how you implement your escaping function. Assuming that your code is perfect at any time is an awful mistake to be making in our trade. This is what testing is for, but safety nets do sometimes fail.

Furthermore, as other people have discussed, there may be scenarios in which your special characters will become outdated due to changes to the DB system you are using. In this case, you will have to update your escaping function or risk being screwed over due to changes that were not entirely in your control. Even though you expect them to be "announced well in advance," assuming that they will be is probably a mistake. Maybe you have more faith in others than I do, but I'm a pretty cynical person and I don't trust other people to notify me of important things.

So really, sure, you could hypothetically write a "perfect" escaping function and have a decent chance of being safe. But it's kind of like riding a motorcycle: You may be an excellent rider, but you still should wear your helmet just in case.

I hope this answers your question a little better.

Okay, to discuss this seriously.

I brought this up mostly because I think people are overstating the difficulty of writing such an escape function, not because it's something I desperately want to do. But writing a SQL escape function is almost trivially simple. For MySQL, a completely thorough, 100% bulletproof, and pretty-well optimized solution would be:

public String qouteString(String x)
{
  if (x == null) {
    return "''"; // or whatever default behavior you want for incoming nulls    
  } else {
    StringBuffer buf = new StringBuffer((int) (x.length() * 1.1));
    buf.append('\'');

    int stringLength = x.length();
    for (int i = 0; i < stringLength; ++i) {
      char c = x.charAt(i);

      switch (c) {
	case 0:
	  buf.append('\\');
	  buf.append('0');
          break;
        case '\n':
          buf.append('\\');
	  buf.append('n');
          break;
        case '\r':
	  buf.append('\\');
	  buf.append('r');
          break;
        case '\\':
	  buf.append('\\');
          buf.append('\\');
          break;
        case '\'':
	  buf.append('\\');
	  buf.append('\'');
          break;
        case '"': /* Better safe than sorry */
	  if (this.usingAnsiMode) {
	    buf.append('\\');
	    }
          buf.append('"');
          break;
        case '\032': /* This gives problems on Win32 */
	  buf.append('\\');
	  buf.append('Z');
          break;
        default:
	  buf.append(c);
	  }
        }
	buf.append('\'');

        return buf.toString();
  }
}

This assumes that your input might includes nulls, new lines, etc. and you want to generate the proper escape sequences, and not merely guard against SQL injection. If you know that's impossible given the source of your input, you could leave out those cases.

I think that would also work in Postgres. I'd have to check for other DBs.

It's not like it's some massively complicated function that would be difficult to test and validate. It's, what, a couple of dozen lines?

Now granted, it's not portable. There are probably DB engines out there that don't treat backslash as a magic character and/or have some other magic character. But it's not like any DB I've ever used has dozens of such magic characters. The only ones I recall seeing are the single quote and the backslash. But you'd have to at least verify it before trying to use it on another db engine. A fair case could be made to say, why bother when you can use prepared statements and not have to worry about it.

Could the db manufacturer decide to add a new magic character? Sure, in principle. But they could decide to change the syntax of the WHERE clause, too. Software vendors tend to be pretty careful to maintain upward compatibility. Any time they break upward compatability, any decent vendor will warn you with bold print in release notes. I'm much more worried about changes to syntax, as that is likely to be spread across many queries. Something like this is going to be in one function, one place to change.

These days I'm mostly using parameterized queries. But I've had times when I've used such a function because I find it more convenient than creating prepared queries and having to set the parameters. It's less code, and because I don't have to specify the name in one place and the value in another, it eliminates some possibility for error.

Maybe you're thinking that there could be some subtle hacker trick that would break the escape function I gave above but would be caught by prepared queries. Maybe you're thinking I was being naive and/or arrogant when I said that this code was 100% bullet-proof. But I wasn't, because I didn't write it. The code above is copied straight out of Oracle's MySQL source. It's the code they execute when you call setString to set a parameter in a PreparedStatement.

2012-02-09 Reply Admin

That's cool, but you don't even have to write one because, as you have so adeptly pointed out, MySQL already does it for you!!! And as an added bonus, they will even update their own internal implementation of the escape function if and when they add magic characters.

I don't think anybody is saying that it's not simple. Because it absolutely is. But the point is that there are better methods out there that make it less desirable and in some cases redundant.

2012-02-09 Reply Admin

dkf:
You should also learn about prepared statements. Basically, the SQL code becomes a template that is substituted into at execution time, with this substitution being done by the database engine rather than your code. It gives very similar speed to stored procedures combined with the flexibility of hand-written SQL (frameworks often only support a small subset of the queries that your app needs). The advantage over escaping is that you don't need to remember to get the escaping done right when you're using prepared statements. (It's even nicer if you have named substitution variables, but that's not universally supported.)

Sorry, I guess I used the terms interchangeably because they both suffer from the same problem -- handwritten SQL. This means many instances of repeating yourself, application fragility and updates that take way longer than they should.

I've found that 49 times out of 50 the framework's automatically generated SQL works just fine thank you very much, and in the very rare cases that you do need some handwritten SQL the framework allows you to enter this as a one-off.

hoodaticus · 2012-02-09 Reply Admin

The poop... of DOOM!:
Sea Sharp:
(...)I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.(...)
And I've written a couple thousand lines of .NET and Java, yet I more often than not don't understand some of the .NET or Java WTFs on here. Not everybody's a genius master programmer of APLEC (All Programming Languages Ever Conceived), so please keep us lowly, non-all knowing developers in mind before blinding us with your all-illuminating greatness.
Captcha: Ideo. Thanks for making us feel like ideots

I've never written a single line of PHP; it's still totally comprehensible as pseudo-code.

This is a site for code-snobs. And you have been snobbed (YHBS).

hoodaticus · 2012-02-09 Reply Admin

Mikey:
It's not about using sprocs to validate your input -- it's about them not being vulnerable to SQLi in the first place.

I once ported an inventory management system from Access to SQL. Some new items came in, and my boss stopped the presses because their descriptions had apostrophes, saying they were illegal characters in the system. I told him, "there are no illegal characters - I'm not an idiot".

hoodaticus · 2012-02-09 Reply Admin

You forgot to mention that:

--

is not a character...

hoodaticus · 2012-02-09 Reply Admin

Jay:
Ninja:
Ok so I think you're really missing the point here (partially because I didn't phrase my response very well).
Hypothetically speaking if you do indeed create a function that perfectly escapes the input strings before executing the SQL query, then chances are that you are fine.

However, there are times when your escape function may not turn out to be as exhaustive as you thought. You may miss some special characters, or overlook some special cases. These exceptions will likely be dependent on how you implement your escaping function. Assuming that your code is perfect at any time is an awful mistake to be making in our trade. This is what testing is for, but safety nets do sometimes fail.

Furthermore, as other people have discussed, there may be scenarios in which your special characters will become outdated due to changes to the DB system you are using. In this case, you will have to update your escaping function or risk being screwed over due to changes that were not entirely in your control. Even though you expect them to be "announced well in advance," assuming that they will be is probably a mistake. Maybe you have more faith in others than I do, but I'm a pretty cynical person and I don't trust other people to notify me of important things.

So really, sure, you could hypothetically write a "perfect" escaping function and have a decent chance of being safe. But it's kind of like riding a motorcycle: You may be an excellent rider, but you still should wear your helmet just in case.

I hope this answers your question a little better.

Okay, to discuss this seriously.

I brought this up mostly because I think people are overstating the difficulty of writing such an escape function, not because it's something I desperately want to do. But writing a SQL escape function is almost trivially simple. For MySQL, a completely thorough, 100% bulletproof, and pretty-well optimized solution would be:
public String qouteString(String x)
{
  if (x == null) {
    return "''"; // or whatever default behavior you want for incoming nulls    
  } else {
    StringBuffer buf = new StringBuffer((int) (x.length() * 1.1));
    buf.append('\'');

    int stringLength = x.length();
    for (int i = 0; i < stringLength; ++i) {
      char c = x.charAt(i);

      switch (c) {
	case 0:
	  buf.append('\\');
	  buf.append('0');
          break;
        case '\n':
          buf.append('\\');
	  buf.append('n');
          break;
        case '\r':
	  buf.append('\\');
	  buf.append('r');
          break;
        case '\\':
	  buf.append('\\');
          buf.append('\\');
          break;
        case '\'':
	  buf.append('\\');
	  buf.append('\'');
          break;
        case '"': /* Better safe than sorry */
	  if (this.usingAnsiMode) {
	    buf.append('\\');
	    }
          buf.append('"');
          break;
        case '\032': /* This gives problems on Win32 */
	  buf.append('\\');
	  buf.append('Z');
          break;
        default:
	  buf.append(c);
	  }
        }
	buf.append('\'');

        return buf.toString();
  }
}
This assumes that your input might includes nulls, new lines, etc. and you want to generate the proper escape sequences, and not merely guard against SQL injection. If you know that's impossible given the source of your input, you could leave out those cases.

I think that would also work in Postgres. I'd have to check for other DBs.

It's not like it's some massively complicated function that would be difficult to test and validate. It's, what, a couple of dozen lines?

Now granted, it's not portable. There are probably DB engines out there that don't treat backslash as a magic character and/or have some other magic character. But it's not like any DB I've ever used has dozens of such magic characters. The only ones I recall seeing are the single quote and the backslash. But you'd have to at least verify it before trying to use it on another db engine. A fair case could be made to say, why bother when you can use prepared statements and not have to worry about it.

Could the db manufacturer decide to add a new magic character? Sure, in principle. But they could decide to change the syntax of the WHERE clause, too. Software vendors tend to be pretty careful to maintain upward compatibility. Any time they break upward compatability, any decent vendor will warn you with bold print in release notes. I'm much more worried about changes to syntax, as that is likely to be spread across many queries. Something like this is going to be in one function, one place to change.

These days I'm mostly using parameterized queries. But I've had times when I've used such a function because I find it more convenient than creating prepared queries and having to set the parameters. It's less code, and because I don't have to specify the name in one place and the value in another, it eliminates some possibility for error.

Maybe you're thinking that there could be some subtle hacker trick that would break the escape function I gave above but would be caught by prepared queries. Maybe you're thinking I was being naive and/or arrogant when I said that this code was 100% bullet-proof. But I wasn't, because I didn't write it. The code above is copied straight out of Oracle's MySQL source. It's the code they execute when you call setString to set a parameter in a PreparedStatement.

TRWTF.

hoodaticus · 2012-02-09 Reply Admin

Matt Westwood:
The enlightened manager moves people to where they are most effective at doing what they're good at. A less well enlightened manager (also known as "a cunting arsebrained fuckwitted pointy-haired COBOL-reject") insists on giving people stuff to work on which they are rubbish at (and in fact don't really like doing), on the grounds that "practice makes perfect" and "I want a team of all-rounders" etc.

Part of management is seeing untapped potential that even your employees don't see in themselves, and dragging them kicking and screaming to become greater than they were.

However, I know the type you're talking about, and I hate them too.

2012-02-09 Reply Admin

Roben:
Why reinvent the wheel, even with RegExps? By using prepared statements you get injection prevention for free...

For a record, MySQL has prepared statement support on v4.1 or later. Although this code doesn't show whether it's using MySQL, LAMP was quite common configuration then, wasn't it?

2012-02-10 Reply Admin

It's a lame-brained, insecure replacement for "$db_escape_string()" which is present in all the major DB varieties, EG: "pg_escape_string()" or "mysql_escape_string()" which replace all this with a single, one-line call.

2012-02-10 Reply Admin

Jerry:
SELECT * FROM CUSTOMER.TRANSACTIONS WHERE ID = _____;
For _____ the hacker sends: 123 OR 4 < 5
No quote, no backslash, no semicolon... must be OK, right?

WHERE id='_____'; noob.

dkf · 2012-02-10 Reply Admin

Nebs:
Sorry, I guess I used the terms interchangeably because they both suffer from the same problem -- handwritten SQL. This means many instances of repeating yourself, application fragility and updates that take way longer than they should.
I've found that 49 times out of 50 the framework's automatically generated SQL works just fine thank you very much, and in the very rare cases that you do need some handwritten SQL the framework allows you to enter this as a one-off.

The only queries that a framework ever can be relied upon to get right (with the exception of the thoroughly-clever LINQ to SQL) are to retrieve an entire row (mapped to an object) or to retrieve all rows (mapped to objects). The first requires that you know the ID already, which is often true but not always, and the second isn't a good idea with a production database with a few million rows (though there have been a number of stories here where that's what has been coded). Other kinds of queries can be done, but they tend to be messy. So queries are often hand-coded; as long as transactions are used sensibly, the cost isn't too high.

Updates, inserts and deletes will all typically work fine when framework-generated. (Guess what the frameworks use internally? Yes, prepared statements.)

2012-02-10 Reply Admin

hoodaticus:
The poop... of DOOM!:
Sea Sharp:
(...)I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.(...)
And I've written a couple thousand lines of .NET and Java, yet I more often than not don't understand some of the .NET or Java WTFs on here. Not everybody's a genius master programmer of APLEC (All Programming Languages Ever Conceived), so please keep us lowly, non-all knowing developers in mind before blinding us with your all-illuminating greatness.
Captcha: Ideo. Thanks for making us feel like ideots
I've never written a single line of PHP; it's still totally comprehensible as pseudo-code.

This is a site for code-snobs. And you have been snobbed (YHBS).

And I should care why? If he gets his giggles from getting replies like mine, then let him giggle.

2012-02-10 Reply Admin

Coyne:
The poop... of DOOM!:
Some Damn Yank:
Guest:
Sure use Regex, much better. Hope that was meant as Joke... Also mysql_real_escape_string is not safe at all by itself.
New here? A favorite pastime at the DWTF is to find a more elegant way to express the original WTF without fixing the root problem. Such as using a regex in place of a long if/elseif/elseif/elseif... mess - but without fixing the root problem. It's one of the reasons I like this site :-)
Captcha: appellatio. An obscene act performed on an apple.
Ok, let's give this a go. This way, you don't even need to check the input!
$sql = "select muguser_id, muguser_directory " . 
       "from mugusers " . 
       "where muguser_active = 1 " . 
       " and muguser_email = '" . $_POST["email"] . "' ";

$result = mysql_query($sql);
if(mysql_insert_id() > 0 ) { // Check for injection
  print("SQL INJECTION DETECTED! YOU DIRTY HACKER, YOU!");
}
else {
  // Shenanigans
}
...and let us count the WTF's:

Basic design allows SQL injection.

The fix only detects inserts, so if the injection attack is a DELETE TABLE X, well, too bad.

The fix only detects the insert injection after the insert was done.("Well, yes, all the horses did escape before we closed the barn door. But at least it's closed.")

Only detects the insert injection if it was the last statement the hacker included in his attack.

Provides a nice confirmation message to the hacker so he knows he's on the right track.

All-in-all, a really good example for the WTF mill. :)

That it only detects the insert injection is due to the nature of an SQL injection. As the name implies, it injects data into your SQL database. Hence the need for checking for insert statements only. If it were a delete statement, it wouldn't be an SQL injection, but an SQL removal.

The only thing I forgot to add, though, is a comment saying how clever this solution is.

2012-02-10 Reply Admin

so what happens if someone with username containing "or" like "ordinary" or "moore" wants to login?

thought.... why use regex when u can just strip or escape.

C-Octothorpe · 2012-02-10 Reply Admin

xsicko:
so what happens if someone with username containing "or" like "ordinary" or "moore" wants to login?
thought.... why use regex when u can just strip or escape.

That's easy. Simply throw a "YourNameIsntFuckingValidSoFuckOffException". I think I recall seeing this in the System.Data namespace once...

SQL MUGging

Leave a comment on “SQL MUGging”