• Watson (unregistered) in reply to The poop... of DOOM!
    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 (unregistered) in reply to dkf
    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 (unregistered) in reply to Ben Jammin
    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 (unregistered) in reply to Tim
    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 (unregistered)

    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 (unregistered) in reply to Nebs
    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 (unregistered)

    I don't like programmers who can't escape their strings properly.

  • le (unregistered) in reply to FragFrog
    FragFrog:
    Bottomline: some principals are so fundamental that there is simply no excuse for not learning them.
    Principles.
  • bob (unregistered)

    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.

  • (cs) in reply to Ben Jammin
    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 (unregistered)

    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 (unregistered) in reply to ¯\(°_o)/¯ I DUNNO LOL

    418 - I'm a teapot

    http://www.askapache.com/htaccess/apache-status-code-headers-errordocument.html#418_Im_teapot

  • olddog (unregistered)
    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 (unregistered)

    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 (unregistered) in reply to Coyne

    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 (unregistered) in reply to Todd Lewis

    how is the prepared statement more safe...you're passing the email (POST) variable without checking what's in it, in both cases.

Leave a comment on “SQL MUGging”

Log In or post as a guest

Replying to comment #:

« Return to Article