SQL MUGging

« Return to Article
  • henke37 2012-02-08 09:02
    It also fails to account for the offending text being the first character of the variable.
  • Nagesh-saki 2012-02-08 09:02
    I could have been frist before, but choose not to pollute the boards
  • TheSHEEEP 2012-02-08 09:12
    Okay, for the non-PHP, non-SQL people among us... could someone please explain what's going on here?
  • JD 2012-02-08 09:12
    I always worry that my code will one day grace these pages, but after reading stories like these, I breathe a little sigh of relief.
  • Roben 2012-02-08 09:13
    Why reinvent the wheel, even with RegExps? By using prepared statements you get injection prevention for free...
  • Some child 2012-02-08 09:13
    "Every child knows this is insecure and that Best Practice would be to harness the power of regular expressions"

    I wwas told that Best Practice is the use of prepared statements but I wouldn't be such a prick to say "every child knows".
  • Roben 2012-02-08 09:15
    Some child:
    I was told that Best Practice is the use of prepared statements but I wouldn't be such a prick to say "every child knows".

    +1 :)
  • JD 2012-02-08 09:17
    What they're doing is checking the username and password for special characters that can be used to escape a SQL statement and inject their own code into the statement.
  • Tim 2012-02-08 09:19
    Some child:
    "Every child knows this is insecure and that Best Practice would be to harness the power of regular expressions"

    I wwas told that Best Practice is the use of prepared statements but I wouldn't be such a prick to say "every child knows".

    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
  • Bert 2012-02-08 09:19
    If only there were some kind of
    mysql_real_escape_string
    function.
  • Crisw 2012-02-08 09:20
    JD:
    What they're doing is checking the username and password for special characters that can be used to escape a SQL statement and inject their own code into the statement.


    What they've done is completely ineffective, but ok.
  • Mario Vilas 2012-02-08 09:20
    Then "every child" must be a moron, because that solution is even worse than the original code... FAIL!
  • Steve The Cynic 2012-02-08 09:23
    Some child:
    "Every child knows this is insecure and that Best Practice would be to harness the power of regular expressions"

    I wwas told that Best Practice is the use of prepared statements but I wouldn't be such a prick to say "every child knows".

    "Every child" does indeed know this gobble about regular expressions. Those of us who have grown up and learned a few things know better, and we use baseball bats^W^Wprepared statements.
  • Sea Sharp, Waves Hurt 2012-02-08 09:25
    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 here, 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.)
  • Sea Sharp, Waves Hurt 2012-02-08 09:33
    Hello, welcome to no edits.

    > "that can just *be read*" -> "that can't just *be read*"
    > "venio, vedio, viccio" -> "venio, vedio, vicio"

    CAPTCHA: similis
    "I don't like Vera's similis 'tude one bit."
  • Guest 2012-02-08 09:34
    Sure use Regex, much better. Hope that was meant as Joke... Also mysql_real_escape_string is not safe at all by itself.
  • Ben Jammin 2012-02-08 09:36
    What I like is that the first case tests for " ", completely negating the last half dozen.
  • Bert 2012-02-08 09:38
    Guest:
    mysql_real_escape_string is not safe at all by itself.

    Yes, but what part of PHP is safe by itself?
  • PedanticCurmudgeon 2012-02-08 09:54
    Sea Sharp, Waves Hurt:
    When someone says, on here, that they aren't familiar with <language> and would like someone who is to explain, what are they actually saying?
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
  • Anon 2012-02-08 09:55
    Mario Vilas:
    Then "every child" must be a moron, because that solution is even worse than the original code... FAIL!


    Whoosh!!
  • dgvid 2012-02-08 09:59
    "Every child knows" that the Internet is killing the art of sarcasm.
  • dgvid 2012-02-08 10:01
    dgvid:
    "Every child knows" that the Internet is killing the art of sarcasm.


    Oops. I forgot to include a meme-injection attack in my comment. How's this?

    My every child was killed by knowledge and I assure it was no laughing matter.
  • ¯\(°_o)/¯ I DUNNO LOL 2012-02-08 10:04
    Steve The Cynic:
    Those of us who have grown up and learned a few things know better, and we use baseball bats^W^Wprepared statements.

    Which HTTP return code hits the user with a baseball bat? This is relevant to my interests.

    And I'm going with "Every child knows..." as being a joke. Because I just couldn't handle it not being a joke.
  • Todd Lewis 2012-02-08 10:10
    TheSHEEEP:
    Okay, for the non-PHP, non-SQL people among us... could someone please explain what's going on here?


    A fair question. If you build your SQL query using strings the user provides, someone will be either clever enough or stupid enough to break your SQL, sometimes intentionally and sometimes in ways that compromise your data/users/site whatever.

    Programmers who don't know better think they can sanitize the inputs and thus create safe SQL from it. They may reduce the window of vulnerability, but there is a better way.

    The better way is "prepared statements." So instead of building SQL code to execute directly like this:
    $sql = "select muguser_id, muguser_directory " . 
    
    "from mugusers " .
    "where muguser_active = 1 " .
    " and muguser_email = '" . $_POST["email"] . "' ";


    you would make a prepared statement:
    $stmt = $dbh->prepare("select muguser_id, muguser_directory " . 
    
    "from mugusers " .
    "where muguser_active = 1 " .
    " and muguser_email = ?");
    if ($stmt->execute(array($_POST['email']))) {
    while ($row = $stmt->fetch()) {
    # do something wonderful;
    }
    }


    This ensures the SQL statements are known text; they aren't built from any bits supplied by user input. Likewise, your user's input is not polluted by strange quoting and string interpolations from your host language (in this case, php).

    This same technique is available in pretty much any language that can make SQL calls.

    And now you have no excuse.
  • Nagesh 2012-02-08 10:21
    I am waiting to see some guy bring out Boby Tables

  • Spannenlangerhansl 2012-02-08 10:23
    Todd Lewis:
    And now you have no excuse.

    +1
  • QJo 2012-02-08 10:24
    I see - this is the code you use to keep track of who still hasn't been sent their WTF mug? Good of you to share.
  • QJo 2012-02-08 10:24
    I had a mug once. It was funny.
  • Kyles 2012-02-08 10:28
    When someone says, on here, 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.


    Unless that language is perl.
  • Sea Sharp, Waves Hurt 2012-02-08 10:43
    PedanticCurmudgeon:
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
    I guess I can think of a few I'd put on that list.

    Kyles:
    Unless that language is perl.
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.
  • RichP 2012-02-08 10:49
    Clever MUGgles, they don't know how to use the magic of mysql_real_escape_string, so they have to resort to ingenious workarounds.
  • trtrwtf returns 2012-02-08 10:54
    Sea Sharp, Waves Hurt:
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've written.


    FTFY
  • Jonathan 2012-02-08 10:58
    Once again, TRWTF is toys like PHP and MySQL for not having bound parameters and prepared statements from the start, and for people using toys like that in production. Not like mysqli, which finally does support prepared statements, hasn't been around for an entire major version of both PHP and MySQL...
  • Charlie 2012-02-08 11:01
    Oh, a photo site, maybe?
  • Tom 2012-02-08 11:05
    Seriously? "Every child knows regular expressions are the answer"...and people don't catch the biting sarcasm?

    Captcha: "suscipere". Yes, I acknowledge the sarcasm.
  • Tom 2012-02-08 11:07
    Kyles:
    When someone says, on here, 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.


    Unless that language is perl.


    Or JCL.

    If one considers JCL An language, and not some horrible misguided practical joke taken to extremes.
  • Paul Neumann 2012-02-08 11:08
    Sea Sharp, Waves Hurt:
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.
    Perl is not it's own paradigm. It is in the same family of languages as BrainF*ck, Taxi, and Piet. Someone just forgot to tell the Perl users it was a joke.

    From a usability standpoint, it is nearly to the level of LOLCODE.

    Yes Akismet, I just learned to [ab]use url tags. No Akismet, this is not spam.
  • Steve The Cynic 2012-02-08 11:11
    QJo:
    I had a mug once. It was funny.

    I have a mug now. It is boring.
  • el_timm 2012-02-08 11:14
    I think the learning curve is on a %2 years:

    0-2: no protection
    2-4: @see OP
    4-6: addslashes()
    6-8: mysql_escape_string()
    8-10: mysql_real_escape_string()
    10+: Become a manager and forget all the above.
  • ekolis 2012-02-08 11:14
    What do they have against SPACES? Surely there's no way you can use a space character to cause a SQL injection...
  • caper 2012-02-08 11:16
    Why would you not use a prepared statement ?
    Where are these people coming from who don't yet know about prepared statements ?
  • neener neener 2012-02-08 11:17
    Sea Sharp, Waves Hurt:

    When someone says, on here, 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.

    From what I gather, a lot of these questions arise because the issue with the code at-hand is some subtle nuance of the specific language or how the language interacts with something not language specific (e.g., a database).
  • C-Octothorpe 2012-02-08 11:20
    Sea Sharp, Waves Hurt:
    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:
    I have an honest question about you questioning the original question: what gave you the idea that everybody here has a programming or technical background?
    Sea Sharp, Waves Hurt:
    When someone says, on here, that they aren't familiar with <language> and would like someone who is to explain, what are they actually saying?
    Um... When someone asks you to pass the sugar, what do you think they're asking?
    Sea Sharp, Waves Hurt:
    I guess I'm asking coming from the position that I can't think beyond myself and am trying to sound smart while doing so.
    FTFY
    Sea Sharp, Waves Hurt:
    I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.
    Congrats, that's 250 lines of PHP more than I and a huge majority of people have ever done. What makes you think that some anecdotal observation isolated to yourself only applies to the whole?

    I don't mean to sound like a troll, but I hate these smug "You're a moran for asking" comments.
  • Sea Sharp, Waves Hurt 2012-02-08 11:21
    Paul Neumann:
    Perl is not it's own paradigm. It is in the same family of languages as BrainF*ck, Taxi, and Piet.
    Well, yes. Not literally :). Given how ridiculous it is, sometimes it seems to be. Forgive my lack of markup on the metaphor.

    neener neener:
    From what I gather, a lot of these questions arise because the issue with the code at-hand is some subtle nuance of the specific language or how the language interacts with something not language specific (e.g., a database).
    I suppose this time it seemed more odd given the code at hand.

    --

    Also, I read this title as "SQL Munging". I guess that's not entirely innacurate.
  • Sea Sharp, Waves Hurt 2012-02-08 11:23
    I think something about this site is making me unable to spell properly... (I swear I'm proofreading before I hit Submit.)

    > "innacurate" -> "inaccurate"
  • Jerry 2012-02-08 11:26
    Bert:
    Guest:
    mysql_real_escape_string is not safe at all by itself.

    Yes, but what part of PHP is safe by itself?
    Any part of PHP is safe by itself. It's when you hook it up to something else, like the Internet, that you're doomed.

    Seriously, prepared statements are so easy, and everyone keeps trying to remind you that they are the right way to do it, so why would anybody say "yeah but why can't I keep trying to build a better black-list filter?"

    Use prepared statements.

    Or stop programming.

    Those are your choices.
  • FragFrog 2012-02-08 12:03
    C-Octothorpe:
    I have an honest question about you questioning the original question: what gave you the idea that everybody here has a programming or technical background?

    I'm going out on a limb here, but the fact that this was posted in the CodeSOD catagory might be a hint? You know, Code Snippet of the Day? Might be directed at people who (know) code? Far fetched, I know..

    Really though, complaining that an article in CodeSOD is not understandable for non-coders is a bit like going to a star-trek forum and asking who this Picard fellow is. Anyone with basic programming skills should know that blacklisting is not a reliable method to prevent SQL injection, and this code is a prime example of why we have so many SQL injection hacks.
  • gabs 2012-02-08 12:18
    y u no
    mysql_real_scape_string($email_or_pwd_or_un)
    ?!
  • Some Damn Yank 2012-02-08 12:20
    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.
  • Rick 2012-02-08 12:29
    Kyles:
    When someone says, on here, 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.


    Unless that language is perl.
    Or APL.
  • PedanticCurmudgeon 2012-02-08 12:31
    Paul Neumann:
    Sea Sharp, Waves Hurt:
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.
    Perl is not it's own paradigm. It is in the same family of languages as BrainF*ck, Taxi, and Piet. Someone just forgot to tell the Perl users it was a joke.

    From a usability standpoint, it is nearly to the level of LOLCODE.

    Yes Akismet, I just learned to [ab]use url tags. No Akismet, this is not spam.
    I know you're trolling, but this is actually funny enough to feature.
  • Some Damn Yank 2012-02-08 12:34
    Tom:
    If one considers JCL An language, and not some horrible misguided practical joke taken to extremes.
    I knew JCL once, but I was clever enough to never put it on my resume. It was actually fun knowing something most of my co-workers would never understand, and the power of being able to write my own code (as opposed to using the canned JCL the mysterious gnomes in the Computer Room fed us) was cool. Not quite awesome, but cool.

    You won't see JCL here, as there's no room for a WTF. It either runs or it doesn't.
  • C-Octothorpe 2012-02-08 12:37
    FragFrog:
    C-Octothorpe:
    I have an honest question about you questioning the original question: what gave you the idea that everybody here has a programming or technical background?

    I'm going out on a limb here, but the fact that this was posted in the CodeSOD catagory might be a hint? You know, Code Snippet of the Day? Might be directed at people who (know) code? Far fetched, I know...
    You're proving my case for me: a shitty, 'holier than thou' attitude prevents any kind of learning for people who may just be interested in programming or like participating in the comments section.
    FragFrog:
    Really though, complaining that an article in CodeSOD is not understandable for non-coders is a bit like going to a star-trek forum and asking who this Picard fellow is.
    I'm not complaining that it's not understandable for non-coders. What I am saying is that if someone is asking a question because maybe the WTF is not completely obvious to them, only to be met with ridicule stiffles learning and sours the environment (this is true in real life or a comments thread).
    FragFrog:
    Anyone with basic programming skills should know that blacklisting is not a reliable method to prevent SQL injection, and this code is a prime example of why we have so many SQL injection hacks.
    This just smacks of ignorance... So because someone has years of experience building device drivers, operating system kernels or software for radiotherapy machines should know all about web-related attack vectors?

    Also, 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.
  • Tasty 2012-02-08 12:40
    Rick:
    Kyles:
    When someone says, on here, 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.


    Unless that language is perl.
    Or APL.


    Or MUMPS.
  • Sea Sharp, Waves Hurt 2012-02-08 12:55
    C-Octothorpe:
    I have an honest question about you questioning the original question: what gave you the idea that everybody here has a programming or technical background?
    Well, I don't think what I wrote implied that. From the OP, "for the non-PHP, non-SQL people among us" -- I admit that I took that to imply that while the OP was "non-PHP, non-SQL", s/he was not a layperson in terms of coding. If s/he was, I'd assume that it would've been phrased as "non-coder". If that was the case, it would make total sense to ask.

    C-Octothorpe:
    Um... When someone asks you to pass the sugar, what do you think they're asking?
    Point taken on that. It was a stupid question as phrased. What I was hoping to ask was something more along the lines of, "What is the root issue/cause implied by the need to ask the question?"

    C-Octothorpe:
    Sea Sharp, Waves Hurt:
    I guess I'm asking coming from the position that I can't think beyond myself and am trying to sound smart while doing so.
    FTFY
    Well, to be fair, I'm trying to get the people here to help me think beyond myself and understand what the issue/cause is. While I can't say that it's true that I can't think beyond myself, I often want to have the out-think happen collaboratively.

    C-Octothorpe:
    Sea Sharp, Waves Hurt:
    I have done maybe 250 lines of PHP in my life and I can grok what's going on here pretty completely.
    Congrats, that's 250 lines of PHP more than I and a huge majority of people have ever done. What makes you think that some anecdotal observation isolated to yourself only applies to the whole?
    You're taking that line out of the context of the intention of the whole post. I'm using that as ruler. If I have only written that much PHP ... the point is not that I have actually ever written PHP. The implication of that statement is that having only ever written that much, I would ostensibly be unfamiliar enough with it that I'm using my general programming syntax/grammar knowledge to understand it. I'm not using my muscle-memory familiarity with PHP. My ability to do the former contrasted with the request I originally quoted was the cause for my post in the first place.

    --

    So... The real question I have, which was reasonably called out as not clear, is: what would prevent a person who is familiar from coding within a particular paradigm from understanding (at least generally -- nuance withstanding, obviously) a piece of code from the same paradigm in a language with which they are not specifically familiar?

    CAPTCHA: paratus
    "Mea argumentum non paratus."
  • Jerry 2012-02-08 12:56
    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
  • C-Octothorpe 2012-02-08 13:15
    Sea Sharp, Waves Hurt:
    So... The real question I have, which was reasonably called out as not clear, is: what would prevent a person who is familiar from coding within a particular paradigm from understanding (at least generally -- nuance withstanding, obviously) a piece of code from the same paradigm in a language with which they are not specifically familiar?
    Gotcha... Realistically, they probably didn't want to appear to know absoultely nothing about programming for fear of ridicule. They probably though that saying "hey, I don't know PHP; whats the WTF" sounds better than "I've never really programmed before. What's the WTF?".
  • PoPSiCLe 2012-02-08 13:27
    Tasty:
    Rick:
    Kyles:
    When someone says, on here, 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.


    Unless that language is perl.
    Or APL.


    Or MUMPS.


    Or Haskell... I'll just leave out the link, and direct anyone who wants to hurt themselves so badly to Google...
  • C-Octothorpe 2012-02-08 13:30
    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.
  • Anketam 2012-02-08 13:39
    RichP:
    Clever MUGgles, they don't know how to use the magic of mysql_real_escape_string, so they have to resort to ingenious workarounds.
    Aye, only we pure bloods are capable of proper programing. MUGgles are an insult on our order and should all be killed.
  • Sea Sharp, Waves Hurt 2012-02-08 13:44
    C-Octothorpe:
    Gotcha... Realistically, they probably didn't want to appear to know absoultely nothing about programming for fear of ridicule. They probably though that saying "hey, I don't know PHP; whats the WTF" sounds better than "I've never really programmed before. What's the WTF?".
    So, what you're saying is that the motivation might have been to avoid getting responses flavored like mine? :)

    CATPCHA: erat
    "QED"
  • PedanticCurmudgeon 2012-02-08 13:45
    PoPSiCLe:
    Tasty:
    Rick:
    Kyles:
    When someone says, on here, 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.


    Unless that language is perl.
    Or APL.


    Or MUMPS.


    Or Haskell... I'll just leave out the link, and direct anyone who wants to hurt themselves so badly to Google...
    Or Forth.

    But I do have an interesting anecdote about Haskell:

    some guy on #haskell:
    I was TA for a C++ programming course aimed at 1st year physics once.
    Some girl asked for help "i wrote pseudo-code but I cannot translate it to C++".
    Her pseudo-code was valid haskell. I cried.

  • Some Damn Yank 2012-02-08 13:54
    Jerry:
    Use prepared statements.

    Or get hacked.

    Those are your choices.
    FIFY
  • big picture thinker 2012-02-08 14:01
    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.
  • Jay 2012-02-08 14:01
    Sea Sharp, Waves Hurt:
    PedanticCurmudgeon:
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
    I guess I can think of a few I'd put on that list.

    Kyles:
    Unless that language is perl.
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.


    I can definitely say that I can't understand some Javascript that I've written myself.
  • Jay 2012-02-08 14:10
    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?
  • Jay 2012-02-08 14:19
    If user names and passwords are not allowed to have embedded spaces, as the first test is apparently enforcing, then all the tests for "or" with a space before or after are redundant. If names and passwords are allowed to have an embedded space and the first test isn't supposed to be there, then the "or" tests would prohibit, say, someone named "george orwell" from using his own name, but most names would pass. This would be rather mysterious to the poor user. It asks for his name, he types it in, and the system replies "SQL INJECTION ATTACK DETECTED! GO AWAY, BOBBY TABLES!"
  • Ninja 2012-02-08 14:29
    Jay:
    If user names and passwords are not allowed to have embedded spaces, as the first test is apparently enforcing, then all the tests for "or" with a space before or after are redundant. If names and passwords are allowed to have an embedded space and the first test isn't supposed to be there, then the "or" tests would prohibit, say, someone named "george orwell" from using his own name, but most names would pass. This would be rather mysterious to the poor user. It asks for his name, he types it in, and the system replies "SQL INJECTION ATTACK DETECTED! GO AWAY, BOBBY TABLES!"


    Exactly why it's a wtf! It prevents what should be acceptable behavior, while not solving the root problem.

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

    The flipside of your question is what is the harm in having prepared statements? There is not much harm in having your database components being aware of the architecture of the database they are connecting to, and if you need to replace the database system then you may likely have to replace your database component with it anyway. It's still a perfectly valid and modular solution, and much less pain than trying to construct the query (in my opinion) anyway.

    I think I may have the best CAPTCHA of the day:

    Mara: Japanese slang for pen0r.
  • Ben Jammin 2012-02-08 14:47
    Jay:
    Sea Sharp, Waves Hurt:
    PedanticCurmudgeon:
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
    I guess I can think of a few I'd put on that list.

    Kyles:
    Unless that language is perl.
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.


    I can definitely say that I can't understand some Javascript that I've written myself.


    Einstein:
    We can't solve problems by using the same kind of thinking we used when we created them.
  • Jerry 2012-02-08 14:51
    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.
    Well, to be pedantic (and that's what this site is all about, right?) we knew that "sanitizing" bad was insufficient as far back as the previous millineum (i.e. 1999 and prior).
  • Jerry 2012-02-08 14:54
    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?
    1. White lists FTW. List what you know is good. Reject the rest.
    2. Black lists WTF. Don't do that.
    3. Even if you know all other magic characters that the db engine recognizes (and you don't) the list will change tomorrow.
    So just stop arguing and do The Right Thing. Please.
  • GuruBOb 2012-02-08 14:58
    Yep, prepared statements and parameter binding all the way...
  • wcw 2012-02-08 15:09
    Is that the same as a parameterized query? I think that's what I did in my homebrew craptacular web 'site'. Hey, it works, and it's never suffered an injection yet.
  • Sea Sharp, Waves Hurt 2012-02-08 15:24
    wcw:
    Is that the same as a parameterized query? I think that's what I did in my homebrew craptacular web 'site'. Hey, it works, and it's never suffered an injection yet.


    More or less. (Maybe I should type more above the link?)

    http://php.net/manual/en/pdo.prepared-statements.php

    (Hello, Akismet! My CAPTCHA was verto.)
    (Hi again, verto sounds like green in Franglish.)
    (What more do you want from me?)
  • Jerry 2012-02-08 15:57
    wcw:
    Is that the same as a parameterized query? I think that's what I did in my homebrew craptacular web 'site'. Hey, it works, and it's never suffered an injection yet.
    How do you know?

    Or do you mean "it's never suffered an injection yet that was so serious it made the evening news and the hackers gloated all over twitter and stuff and I had to pay out $9 million to the victims of all the compromised records plus nobody trusts me any more."
  • Sole Purpose of Visit 2012-02-08 17:28
    Jay:
    Sea Sharp, Waves Hurt:
    PedanticCurmudgeon:
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
    I guess I can think of a few I'd put on that list.

    Kyles:
    Unless that language is perl.
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.


    I can definitely say that I can't understand some Javascript that I've written myself.


    Punks!

    I can definitely say that I can't understand the Javascript written out by a Perl program I wrote myself, and I can't understand the Perl either. What's more, I'm fairly certain the Perl program was supposed to emit Haskell in the first place...
  • Norman Diamond 2012-02-08 18:21
    Some Damn Yank:
    You won't see JCL here, as there's no room for a WTF. It either runs or it doesn't.
    Oops. Consider someone who wanted to list the members of SYS1.LINKLIB and forgot to specify DISP=SHR.
  • Norman Diamond 2012-02-08 18:35
    Every problem child, faced with self, says: "I know what to do. I'll use regular expressions!"

    Then problem children have twins.
  • mthamil 2012-02-08 18:36
    caper:
    Why would you not use a prepared statement ?
    Where are these people coming from who don't yet know about prepared statements ?


    This is a humor site. It has jokes.
  • olddog 2012-02-08 19:42

    // 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
  • FragFrog 2012-02-08 20:01
    C-Octothorpe:
    Also, 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.
    Really? My apologies if I sounded overly condescending, but is it really too much to ask that a developer knows at least the basics of his trade?

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

    Maybe I do have a 'shitty, holier than thou' attitude. But if so, only because I too made this mistake, many years ago. I learned from that, but not before thousands of users lost their data. I know first hand how dangerous it can be to work on something without understanding the fundamentals. Which is why, in my opinion, anyone working as a (web)developer should also pass some form of exam, demonstrating at least a basic understanding of things like input validation.

    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 a user is able to enter the intensity of radiation. The program expects a dot decimal separator, but the user enters a comma. The program ignores the comma and 5,10J becomes 510J and the patient is fried. So you blacklist? Great, any input containing a comma is rejected. Now a physician who used a different machine enters 5:10J, it is again interpreted as 510J and the patient is once again fried. You keep adding different characters to the blacklist, and people keep getting fried because the input is in some different format (someone used a different character encoding, or wrote in chinese, etc). If you really thing that proper input validation is limited to webdevelopment, I fear you are the ignorant one here.

    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.
  • Hulbert 2012-02-08 20:57
    I read that last line as being sarcastic. Im giving the author the benefit of the doubt, I'm sure they realised there are built in PHP functions for escaping SQL strings.

    Captcha: Tristique - sadly, sorrowfully (like the feeling you get after rewriting a library function)
  • fred 2012-02-08 21:18
    Jay:
    Sea Sharp, Waves Hurt:
    PedanticCurmudgeon:
    My guess is that they're saying that they're not familiar with <language> and would like to keep it that way.
    I guess I can think of a few I'd put on that list.

    Kyles:
    Unless that language is perl.
    Perl is its own paradigm. More than that, it's its own philosophical system. I can definitely say that I can't understand some Perl I've seen.


    I can definitely say that I can't understand some Javascript that I've written myself.


    I have to 2nd that. Javascript code has the magical ability to confuse even its writer, with comments in place (nested callbacks, anyone?)

    On another note, that code just seems like the norm here, after surfing TDWTF for awhile.
  • gizmore 2012-02-08 21:27
    ' OR strpos LIKE 0 anyone? :)
  • Jimmy 2012-02-08 22:32
    FragFrog:
    C-Octothorpe:
    I have an honest question about you questioning the original question: what gave you the idea that everybody here has a programming or technical background?

    I'm going out on a limb here, but the fact that this was posted in the CodeSOD catagory might be a hint? You know, Code Snippet of the Day? Might be directed at people who (know) code? Far fetched, I know..

    Really though, complaining that an article in CodeSOD is not understandable for non-coders is a bit like going to a star-trek forum and asking who this Picard fellow is. Anyone with basic programming skills should know that blacklisting is not a reliable method to prevent SQL injection, and this code is a prime example of why we have so many SQL injection hacks.
    My turn to go out on a limb (this tree is getting mighty top-heavy)...

    A lot of the people who visit this site are WebDevelopers, not Programmers. They may think they are capable of reading hte codeSOD articles but need some explaining.
  • mig 2012-02-08 22:41
    Jay:
    If user names and passwords are not allowed to have embedded spaces, as the first test is apparently enforcing, then all the tests for "or" with a space before or after are redundant. If names and passwords are allowed to have an embedded space and the first test isn't supposed to be there, then the "or" tests would prohibit, say, someone named "george orwell" from using his own name, but most names would pass. This would be rather mysterious to the poor user. It asks for his name, he types it in, and the system replies "SQL INJECTION ATTACK DETECTED! GO AWAY, BOBBY TABLES!"
    if you're consistent, wgo cares?

    George Orwell is stored as GeorgeOrwell (with the space removed) and this is what is subsequently checked.....
  • gatesy 2012-02-08 22:46
    Jerry:
    wcw:
    Is that the same as a parameterized query? I think that's what I did in my homebrew craptacular web 'site'. Hey, it works, and it's never suffered an injection yet.
    How do you know?

    Or do you mean "it's never suffered an injection yet that was so serious it made the evening news and the hackers gloated all over twitter and stuff and I had to pay out $9 million to the victims of all the compromised records plus nobody trusts me any more."
    Uhm...noone visiting your site makes it nice and secure....?
  • olddog 2012-02-08 23:04
    The problem with old programers who think they know more than web-developers is usually evident on their first web site effort. The old programers are the ones scrambling to to find a teenager who understands Javascript and CSS.
  • hoodaticus 2012-02-09 00:44
    You had me at the first code block. I guess idiots speak every language.
  • Nebs 2012-02-09 00:44
    Jerry:


    Seriously, prepared statements are so easy, and everyone keeps trying to remind you that they are the right way to do it, so why would anybody say "yeah but why can't I keep trying to build a better black-list filter?"

    Use prepared statements.

    Or stop programming.

    Those are your choices.


    I can't believe so many people are advocating the use of stored procedures. What is it with you guys? If you're using them to validate your input, then you've got some serious issues. Perhaps they might be appropriate if you've never heard of frameworks, and blindly assume that you will never have to maintain your website after it's been built, but really they are far more pain than they are worth.

    I've been dealing with a PHP project that uses stored procedures (hundreds and hundreds of them) for the last two years, which has proven to me that the concept is completely flawed for PHP programming. Not only do you have many issues with repeating yourself ad-nauseum (which means that if you try to modify any part of the database you have to modify loads of stored procedures, you invariably miss one which breaks the client site when it goes live) but if you write the stored procedure to the website as the wrong user then you won't be able to dump the database later on as a different user.


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

    1. (very bad) Handwritten SQL inside PHP
    2. (bad) Handwritten SQL with mysql-real-escape-string()
    3. (bad) Handwritten SQL inside stored procedures
    4. (excellent) Automatically generated SQL made by a framework

    As an indication, I can write a simple CRUD admin system for a database table in around 60 minutes if using stored procedures, and 1 minute if using my framework of choice. Also the framework one is more secure, and quicker to update later on.


    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.
  • hoodaticus 2012-02-09 00:52
    FragFrog:
    C-Octothorpe:
    Also, 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.
    Really? My apologies if I sounded overly condescending, but is it really too much to ask that a developer knows at least the basics of his trade?

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

    Maybe I do have a 'shitty, holier than thou' attitude. But if so, only because I too made this mistake, many years ago. I learned from that, but not before thousands of users lost their data. I know first hand how dangerous it can be to work on something without understanding the fundamentals. Which is why, in my opinion, anyone working as a (web)developer should also pass some form of exam, demonstrating at least a basic understanding of things like input validation.

    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 a user is able to enter the intensity of radiation. The program expects a dot decimal separator, but the user enters a comma. The program ignores the comma and 5,10J becomes 510J and the patient is fried. So you blacklist? Great, any input containing a comma is rejected. Now a physician who used a different machine enters 5:10J, it is again interpreted as 510J and the patient is once again fried. You keep adding different characters to the blacklist, and people keep getting fried because the input is in some different format (someone used a different character encoding, or wrote in chinese, etc). If you really thing that proper input validation is limited to webdevelopment, I fear you are the ignorant one here.

    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.
    Just because you need undergrad-level instruction doesn't mean we all do. Competence means circumspection and a shitload of reading. Don't fuck up the field by regulating it all to hell.
  • hoodaticus 2012-02-09 00:54
    olddog:
    The problem with old programers who think they know more than web-developers is usually evident on their first web site effort. The old programers are the ones scrambling to to find a teenager who understands Javascript and CSS.
    Spoken like a web "developer"; I've never met one that can multithread.
  • Matt Westwood 2012-02-09 01:28
    hoodaticus:
    FragFrog:

    tl;dr

    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.
    Just because you need undergrad-level instruction doesn't mean we all do. Competence means circumspection and a shitload of reading. Don't fuck up the field by regulating it all to hell.

    News for you bubba, the field is *already* fucked up.
  • Matt Westwood 2012-02-09 01:34
    Nebs:
    Jerry:


    Seriously, prepared statements are so easy, and everyone keeps trying to remind you that they are the right way to do it, so why would anybody say "yeah but why can't I keep trying to build a better black-list filter?"

    Use prepared statements.

    Or stop programming.

    Those are your choices.


    I can't believe so many people are advocating the use of stored procedures. What is it with you guys? If you're using them to validate your input, then you've got some serious issues. Perhaps they might be appropriate if you've never heard of frameworks, and blindly assume that you will never have to maintain your website after it's been built, but really they are far more pain than they are worth.

    I've been dealing with a PHP project that uses stored procedures (hundreds and hundreds of them) for the last two years, which has proven to me that the concept is completely flawed for PHP programming. Not only do you have many issues with repeating yourself ad-nauseum (which means that if you try to modify any part of the database you have to modify loads of stored procedures, you invariably miss one which breaks the client site when it goes live) but if you write the stored procedure to the website as the wrong user then you won't be able to dump the database later on as a different user.


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

    1. (very bad) Handwritten SQL inside PHP
    2. (bad) Handwritten SQL with mysql-real-escape-string()
    3. (bad) Handwritten SQL inside stored procedures
    4. (excellent) Automatically generated SQL made by a framework

    As an indication, I can write a simple CRUD admin system for a database table in around 60 minutes if using stored procedures, and 1 minute if using my framework of choice. Also the framework one is more secure, and quicker to update later on.


    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.
  • Nappy 2012-02-09 02:10
    Beware of the clan!!!
    http://obrienclan.com/
  • Mikey 2012-02-09 02:11
    Real WTF is that you're building a DB query using string concatenation in the first place. Why can't a username be "OR'"? You should be using stored procedures and parameter passing, not concatenating strings. -- I really hope you're validating against HTMLi/JSi as well... I'd guess you are not from the code snippet.
  • Mikey 2012-02-09 02:14
    It's not about using sprocs to validate your input -- it's about them not being vulnerable to SQLi in the first place.
  • Stev 2012-02-09 04:16
    I have a good grounding in C-like languages, such as C++ and Java and I know a bit of other languages (lua, BASIC, etc.) but I stay the hell away from anything remotely webby, like PHP and even anything beyond simple HTML because it's not something I'm particularly interested in - so for me, this WTF wasn't completely obvious, although from the context I had an idea as to what the WTFer was trying to do and didn't - I just didn't know why.

    I'm thankful to the people who have explained it, I've learned a little something and if 5 or 10 years down the line I happen to be asked to "take a look at" some PHP, there's a chance this little nugget will pop up and I'll have a vague idea where to look, rather than just going "hurrr, dunno anything about PHP". There is absolutely no shame in asking questions about something you don't understand, especially when the site in question revolves around the people who don't understand it in the first place.

    It's for the same reason that quip about regex went over so many people's heads - if you just don't know PHP, you're sure as hell not going to know best practices in PHP.
  • milleniumbug 2012-02-09 04:45
    Really? YOU have issues - PREPARED STATEMENTS are not STORED PROCEDURES. Prepared statements are completely different.

    PHP Data Object
    Google it.
  • SEMI-HYBRID code 2012-02-09 06:33
    ...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).
  • dkf 2012-02-09 06:56
    Nebs:
    In order of how problematic different approaches are, I would rate the types as follows:

    1. (very bad) Handwritten SQL inside PHP
    2. (bad) Handwritten SQL with mysql-real-escape-string()
    3. (bad) Handwritten SQL inside stored procedures
    4. (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.)
  • Brendan 2012-02-09 07:16
    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.
  • Danny 'Rushyo' Moules 2012-02-09 07:35
    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).
  • Ralph 2012-02-09 07:43
    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 07:44
    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.
  • frederik 2012-02-09 07:55
    Ah, the days that a DailyWTF could be remotely interesting. Not anymore.
  • no laughing matter 2012-02-09 07:57
    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; --
  • Marius 2012-02-09 08:00
    Indeed it's not. mysql_no_seriously_we_got_it_right_this_time() is the way to go
  • "Location", not "location" 2012-02-09 08:19
    TRWTF is that the "Location" header used to redirect to index.html is lowercased
  • AdT 2012-02-09 08:46
    "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
  • The poop... of DOOM! 2012-02-09 09:07
    Nagesh:
    I am waiting to see some guy bring out Boby Tables


    Why is that guy hooked up to a Super Nintendo?
  • The poop... of DOOM! 2012-02-09 09:13
    Sea Sharp, Waves Hurt:
    (...)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 09:19
    The poop... of DOOM!:
    Sea Sharp, Waves Hurt:
    (...)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.
  • The poop... of DOOM! 2012-02-09 09:41
    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
    }
  • The poop... of DOOM! 2012-02-09 09:58
    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.
  • Urza9814 2012-02-09 10:38
    Sea Sharp, Waves Hurt:
    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.
  • L. 2012-02-09 10:43
    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

    1. it's totally possible to clean your input to require *no* prep. stmt or stored proc.

    2. 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.)

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

    4. When you don't know why it's great, don't go sayin' everyone should do it and it's the best solution

    5. you damn fools.
  • Micke 2012-02-09 11:15
    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 11:37
    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:

    1) Basic design allows SQL injection.

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

    3) 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.")

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

    5) 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. :)
  • Ben Jammin 2012-02-09 13:32
    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.

    1) Learn sql

    2) Discover frameworks that will automatically generate your sql for you *yay shiny toys that cut my development time to nothing*

    3) 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.

    4) 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.

    5) 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.
  • big picture thinker 2012-02-09 13:46
    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
  • Jay 2012-02-09 14:03
    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.
  • Jay 2012-02-09 14:29
    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?
  • Jay 2012-02-09 14:32
    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.
  • Jerry 2012-02-09 14:40
    SELECT * FROM CUSTOMER.TRANSACTIONS WHERE ID = _____;

    For _____ the hacker sends: 123 OR 4 < 5

    No quote, no backslash, no semicolon... must be OK, right?
  • Jay 2012-02-09 14:49
    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 "&amp;", 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 15:19
    Anketam:
    The poop... of DOOM!:
    Sea Sharp, Waves Hurt:
    (...)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 15:24
    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.
  • Ninja 2012-02-09 15:45
    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.
  • some EMT 2012-02-09 15:57
    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.
  • some EMT 2012-02-09 15:59
    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 16:09
    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?
  • fred 2012-02-09 16:16
    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:
    1) We are not security experts and don't necessarily understand where threats are coming from and how
    2) 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)
    3) 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.
  • Xythar 2012-02-09 18:23
    A whole lot of people commenting on this post seem to have a complete inability to grasp sarcasm.
  • Jay 2012-02-09 18:54
    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.

  • Ninja 2012-02-09 19:06
    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.
  • Nebs 2012-02-09 19:45
    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 20:25
    The poop... of DOOM!:
    Sea Sharp, Waves Hurt:
    (...)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 20:58
    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 21:01
    You forgot to mention that:

    --

    is not a character...
  • hoodaticus 2012-02-09 21:04
    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 21:12
    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.
  • Cheong 2012-02-09 22:14
    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?
  • Ben the SQL god of all that is great 2012-02-10 02:12
    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.
  • L. 2012-02-10 05:06
    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 07:21
    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.)
  • The poop... of DOOM! 2012-02-10 08:24
    hoodaticus:
    The poop... of DOOM!:
    Sea Sharp, Waves Hurt:
    (...)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.
  • The poop... of DOOM! 2012-02-10 08:28
    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:

    1) Basic design allows SQL injection.

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

    3) 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.")

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

    5) 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.
  • xsicko 2012-02-10 08:55
    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 10:20
    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...
  • Watson 2012-02-10 10:24
    The poop... of DOOM!:
    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.

    Then again, back before the days of Wikipedia, PHP's database support didn't have prepared statements. Instead, PHP protected you with its Magic Quotes™.
  • Ben Jammin 2012-02-10 10:25
    dkf:
    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.)


    Run a trace on an update statement from your thoroughly clever Linq to Sql. It writes writes individual update statements for each of the records in your set. Databases are much quicker at running one statement that updates 100 records rather than 100 statements that modify one record each.
  • callcopse 2012-02-10 11:30
    Ben Jammin:
    dkf:
    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.)


    Run a trace on an update statement from your thoroughly clever Linq to Sql. It writes writes individual update statements for each of the records in your set. Databases are much quicker at running one statement that updates 100 records rather than 100 statements that modify one record each.


    Straw man? L2S will issue single statements as programmed, as this is its primary purpose. However it allows you to create batch statements through expression trees (and probably other ways too) as below:

    http://www.aneyfamily.com/terryandann/post/2008/04/Batch-Updates-and-Deletes-with-LINQ-to-SQL.aspx

    Of course I'll be the first to admit to breaking out the SQL as needed when I figure it's a better bet, but L2S or EF are great bets in my book for taking out a fair bit of donkey work.
  • Shannon O'Neil 2012-02-10 13:05
    Tim:
    Some child:
    "Every child knows this is insecure and that Best Practice would be to harness the power of regular expressions"

    I wwas told that Best Practice is the use of prepared statements but I wouldn't be such a prick to say "every child knows".

    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

    Read outloud to self with Irish accent:

    Stripping out valid characters tis not the answer as ye prevent half of Ireland from entering their proper surname.

    Proper escaping will work most times, prepared statements will always work.

    For the last time, my name is not ONeil!
  • Miss Tuffet's Puppet 2012-02-10 14:53

    What does this have to do with the rings around uranus and being friskted?

    cptchah -conventio- conventional in and out put
  • Sole Reason For Visit 2012-02-11 12:14
    Nebs:


    I can't believe so many people are advocating the use of stored procedures. What is it with you guys? If you're using them to validate your input, then you've got some serious issues. Perhaps they might be appropriate if you've never heard of frameworks, and blindly assume that you will never have to maintain your website after it's been built, but really they are far more pain than they are worth.

    I've been dealing with a PHP project that uses stored procedures (hundreds and hundreds of them) for the last two years, which has proven to me that the concept is completely flawed for PHP programming.


    Let's assume your proof is correct. Have you considered replacing the language, rather than the use of prepared statements (I'll kindly assume that this is what you meant by "stored procedures")?

    Nebs:
    Not only do you have many issues with repeating yourself ad-nauseum (which means that if you try to modify any part of the database you have to modify loads of stored procedures, you invariably miss one which breaks the client site when it goes live)


    Well, perhaps you do mean "stored procedures." Perhaps, in that case, you could explain where a "stored procedure" is horrible and brittle and a "PHP procedure" is not?

    Nebs:
    but if you write the stored procedure to the website as the wrong user then you won't be able to dump the database later on as a different user.


    It's that naughty "security" thing getting in the way again, isn't it?

    Of course, you could always ask a DB Admin to do the dump for you. Which is the rather more common case, and doesn't have anything at all to do with "stored procedures" or "prepared statements."

    Nebs:
    As an indication, I can write a simple CRUD admin system for a database table in around 60 minutes if using stored procedures, and 1 minute if using my framework of choice. Also the framework one is more secure, and quicker to update later on.


    One table? CRUD? OK, fine. But try anything more complicated, with user input and calculations and string concatenation and stuff, and you are going to blow your foot off.

    Let me remind you of the universal dictum: Get It Provably Correct first. Then worry about optimizations.

    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.


    Up a couple of levels? Nice idea. "Hey boss, we just lost $1 million on a SQL injection. But, on the other hand, I'm l33t!"
  • Brupje 2012-02-13 02:50
    I don\'t like programmers who can\'t escape their strings properly.
  • le 2012-02-13 11:45
    FragFrog:

    Bottomline: some principals are so fundamental that there is simply no excuse for not learning them.

    Principles.
  • bob 2012-02-13 20:11
    Only an idiot would use a regexpression to prevent an SQL injection attack.

    SQL injection attacks are best targeted and negated in the SQL , not by trying to sanitize the input in a half assed way.
    A good start would be the use of SQL bind variables, instead of string concatenation.
  • Coyne 2012-02-14 17:38
    Ben Jammin:

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

    1) Learn sql

    2) Discover frameworks that will automatically generate your sql for you *yay shiny toys that cut my development time to nothing*

    3) 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.

    4) 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.

    5) 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.


    There's a compromise, though. What is helpful is a framework that allows you to write the SQL, then does the parametrization and wrapping for you, to give you access classes.

    We use a framework that creates a wrapper class around a user-specified query, so that selects basically become something like:


    QueryTypeClass c = new QueryTypeClass(connection);
    c.select(var1, var2, var3);
    if (c.isOk()) {
    resultVar = c.getColumn();
    } else {
    // didn't find a row (or whatever)
    }


    That's just a sketch, but the idea should be clear. The point is that what you're (rightfully) rejecting is frameworks that try to help you too much.

    Computers are dumb. Good SQL takes intelligence. A good wrapper class around good SQL can be groveled out by something dumb, and help enormously with the interface between good code and good SQL.
  • Dr Doom 2012-02-17 05:27
    I thought this was a best practice in the PHP community? It certainly is with all the Pretend Home Programming aficionados I've had the misfortune to encounter.
  • anon coward 2012-02-20 08:52
    418 - I'm a teapot

    http://www.askapache.com/htaccess/apache-status-code-headers-errordocument.html#418_Im_teapot
  • olddog 2012-02-21 21:23

    The wood table approach always defeats SQL injection.

    First we need to create a wood table database.

    CARVE INTO ".$table." ETCH VALUES ($values=array())

    we now have a tabletop database with all the records engraved into it, each record on its own wood plank.


    next we create a query by engraving the match values into a fresh blank plank.

    CARVE QUERY ETCH VALUES ($values=array())

    next we make a reverse mold of the query plank to use for comparing engraved records.

    CAST MOLD FROM QUERY ( $query, $substance=CONST_HARD_RUBBER, $shape=CONST_ROUNDED_BEVEL )

    We now have an exact plank mold of the plank record we need to match. In this example, we are using CONST_ROUNDED_BEVEL to allow the mold to easily slide in and out of a match, enabling it to check subsequent planks. We could use CONST_HARD_STUCK, if we wanted to stop at the first match. Yes - this engine has some nice features.

    In the next steps, we will manually slide the mold from plank to plank, across the table to see if it "drops" into position - indicating a match.

    First we prepare the table by pouring a small amount of dye into each engraved depression on the table database. As the mold hovers over a plank record, and a match is found, the mold plank will drop (thanks to gravity) into the engraved record, causing it to squeeze out the dye, leaving proof of a match. For Best-Practice results, we reccommed using journeymen database admistrators to perform all operations.

    Depending on your configuration you might use an alternate engine. For this example we will use block and tackle with plenum rated twisted pair cable to slide the mold across the table. One end of the cable is attached to an eye-hook on the mold, the other end to a hand operated winch - a hundred feet away from the table. Our example table has air-holes, much like an air-hockey table to provide a cushion of air for the mold to float across - a much recommended feature. You can of course simply grease the table with Cisco Oil to prevent drag friction - just saying.

    To record the matches, we will use our journeymen database admistrators, specially trained to traverse the table, as the mold iterates each plank. The journeymen are trained to "Ouija-board" the mold at each plank in order to faciliate an "OR", also to make a paper rubbing off each match.

    Okay - we're ready.

    HEAVE HOE ()

    This sets everything in motion. A big heavy lever at the end of the table is heave-hoe'd into a position that releases the cog that locks the winch that pulls the mold across the table. If you have the optional air-compressor-with-bag-pipe installed, a loud obnoxious chord is blown - signaling the journeymen to start working. It may take some days, depending on the number of planks, for the mold to get to the end of the table, and the rubbings to be gathered (and ORDER'd) by the journeymen. This delay provides a convenient interval for cleaning the table - to ready it for the next query.


    To cache the query, we simply don't destroy the mold - how easy is that?

    To delete a record we simply turn the table plank upside down - think undo.

    To faciliate an "AND" condition, additional molds are simply daisy-chained together

    To this day, no sql injection has ever been effective against the wood table.

  • Richard 2012-02-24 11:55
    Every developer should know that parameterised queries are the way to go. Good that PHP supports them.

    I once tried to sign up to a web site of a company that claimed to be an expert in computer security. The site rejected my application form because my name "looked like an SQL injection attack". My name, "Corfield". We narrowed it down to the word "field", then decided not to talk to the company in question about web security.
  • Richard 2012-02-24 12:00
    It depends on your developers. We had an in-house framework for SQL generation and yes it was inefficient, but productivity was really good. Thing is, we then spent some time improving it. Now the generated SQL works really well and we have some nice speed benefits, but the hand coded stuff did not benefit.

    We also benefit from things like Unit of Work, which lets us perform JDBC batching which really helps.

    I've also worked with stored procedures, lower layers in PLSQL. Each to their own I suppose. I value the developer productivity of the object mapping layer, and have seen it work on some quite high volume systems.
  • free 2012-09-26 10:16
    how is the prepared statement more safe...you're passing the email (POST) variable without checking what's in it, in both cases.