SitePoint Sponsor

User Tag List

Page 2 of 2 FirstFirst 12
Results 26 to 48 of 48
  1. #26
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DrQuincy View Post
    Thanks, so I guess is_numeric is the way to go then.
    is_numeric() is the simplest general check that will work fine, you can also use regular expressions (preg_match) when you want to target some more specific number formats. ctype_digit() is also worth considering.

    The one weird thing about is_numeric() is that it will accept a number if it contains any number of white space characters in front of the number. However, this should not cause any issues in sql. If you want to avoid this just remember to use trim() on the value.

    Quote Originally Posted by DrQuincy View Post
    Do you see prepared statements as only useful if you are reusing the queries then?
    Basically, yes. In most cases there some trade-offs I don't think are worth accepting:
    - with prepared statements it is difficult to log queries with actual data (for any purpose) because you send queries with placeholders only and data is sent separately
    - binding values break the linear flow of code as compared to injecting values to sql by the use of concatenating - the sql is in a different place than the bind() statements, so when you look at sql you don't see where the values come from because there are only placeholders. This becomes tedious if a query is larger and you have to scroll your code to see what value/variable is behind the placeholder. Placeholders look cleaner at first glance but after a few times of using them I find the 'uglier' concatenation to be actually easier to read and debug.

  2. #27
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,268
    Mentioned
    18 Post(s)
    Tagged
    0 Thread(s)
    Hi, folks. Sorry I'm late to the party. I wanted to ask about the original issue.

    Quote Originally Posted by DrQuincy View Post
    ...have just read that [escaped numerical values] could still lead to a SQL injection...
    Has anyone provided a testable and repeatable demonstration of this? Because if your values -- numeric or otherwise -- are quoted and escaped, then the vulnerability isn't at all obvious to me.
    "First make it work. Then make it better."

  3. #28
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    Hi, folks. Sorry I'm late to the party. I wanted to ask about the original issue.



    Has anyone provided a testable and repeatable demonstration of this? Because if your values -- numeric or otherwise -- are quoted and escaped, then the vulnerability isn't at all obvious to me.
    While you could quote and escape a numerical as MySQL will convert it to a number if it can, I would consider this bad practice—and doesn't it prevent indexing doing it this way?

    The example I read would go something like:

    $safe = mysql_real_escape_string($_GET['page']);

    In this case $_GET['page'] = "0 = 0"

    So the query could be:

    SELECT ... WHERE somefield = 0 = 0

    Which would return every record.

  4. #29
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lemon Juice View Post
    is_numeric() is the simplest general check that will work fine, you can also use regular expressions (preg_match) when you want to target some more specific number formats. ctype_digit() is also worth considering.

    The one weird thing about is_numeric() is that it will accept a number if it contains any number of white space characters in front of the number. However, this should not cause any issues in sql. If you want to avoid this just remember to use trim() on the value.



    Basically, yes. In most cases there some trade-offs I don't think are worth accepting:
    - with prepared statements it is difficult to log queries with actual data (for any purpose) because you send queries with placeholders only and data is sent separately
    - binding values break the linear flow of code as compared to injecting values to sql by the use of concatenating - the sql is in a different place than the bind() statements, so when you look at sql you don't see where the values come from because there are only placeholders. This becomes tedious if a query is larger and you have to scroll your code to see what value/variable is behind the placeholder. Placeholders look cleaner at first glance but after a few times of using them I find the 'uglier' concatenation to be actually easier to read and debug.
    You've got me questioning whether I should moved to prepared statements now! I tend to agree with everything you've said. With MySQLi is there anyway to get the actual query executed as a string in case of error? If you can't I would have this down as a disadvantage since logging the query upon error is very useful for debugging.

  5. #30
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,268
    Mentioned
    18 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DrQuincy View Post
    While you could quote and escape a numerical as MySQL will convert it to a number if it can, I would consider this bad practice—and doesn't it prevent indexing doing it this way?
    Either PHP or MySQL will eventually have to convert the value to a number. Is it so bad if we let MySQL handle that job? It comes with the added benefit that you can escape those values just like any other value.

    I don't see how this would prevent indexing.
    "First make it work. Then make it better."

  6. #31
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DrQuincy View Post
    While you could quote and escape a numerical as MySQL will convert it to a number if it can, I would consider this bad practice—and doesn't it prevent indexing doing it this way?
    It shouldn't prevent indexing but again I've come across some edge cases involving very large numbers where quotes actually changed what the query did, unfortunately I can't remember the details. Quoting numbers is non-standard and although nothing wrong should happen in mysql I prefer to always use them unquoted. MySQLi's real_escape_string() is very basic and not very universal so what I do is I use my own escaping function that can handle strings (even those from objects with __toString()), numbers, NULLs and also arrays for IN() clause. Actually, this is a method in an extended MySQLi class:
    PHP Code:
    class Db extends mysqli {
        
    /**
         * Escape value for sql query
         * @param mixed $value
         * @param string $type 's'=string, 'n'=number, null=automatic detection based on $value PHP type
         * @return string 
         */
        
    public function quote($value$type null) {
            if (
    $value === null) {
                return 
    "NULL";
                
            } elseif (
    is_array($value)) {
                
    // comma-separated list of values for IN()
                
    $items = array();
                
                foreach (
    $value as $item) {
                    if (
    $item === null) {
                        
    $items[] = "NULL";
                    } elseif (
    $type === "s" || ($type === null && (is_string($item) || is_bool($value)))) {
                        
    // string
                        
    $items[] = "'"$this->real_escape_string($item). "'";
                        
                    } elseif (
    $type === "n") {
                        
    // number
                        
    if (is_numeric($item)) {
                            
    $items[] = $item;
                        } elseif (
    is_object($item)) {
                            
    $items[] = (float) $item;
                        } elseif (
    $item === true) {
                            
    $items[] = '1';
                        } else {
                            
    $items[] = '0';
                        }
                    
                    } elseif (
    $type === null && (is_int($item) || is_float($item))) {
                        
    // auto-detected number
                        
    $items[] = $item;

                    } else {
                        throw new 
    Exception("Db: Wrong IN() parameters for quote()");
                    }
                }
                
                return 
    implode(","$items);
            
            } elseif (
    $type === "s" || ($type === null && (is_string($value) || is_bool($value) || is_object($value)))) {
                
    // string
                
    return "'"$this->real_escape_string($value). "'";
                
            } elseif (
    $type === "n") {
                
    // number
                
    if (is_numeric($value)) {
                    return 
    $value;
                } elseif (
    is_object($value)) {
                    return (float) 
    $value;
                } elseif (
    $value === true) {
                    return 
    '1';
                } else {
                    return 
    '0';
                }
                
            } elseif (
    $type === null && (is_int($value) || is_float($value))) {
                
    // auto-detected number
                
    return $value;
            }
                
            throw new 
    Exception("Db: Wrong parameters for quote(): $value,$type");
        }    

    This may not be exactly what you want so take is as an inspiration. For PDO a similar method could be constructed.

    Quote Originally Posted by DrQuincy View Post
    You've got me questioning whether I should moved to prepared statements now! I tend to agree with everything you've said. With MySQLi is there anyway to get the actual query executed as a string in case of error? If you can't I would have this down as a disadvantage since logging the query upon error is very useful for debugging.
    I don't think you can get the query with data either in Mysqli or in PDO. Even MySQL slow query log will not show the data, as far as I know. You'd have to construct your own mechanisms that would capture data from all bind...() calls along with the prepared statement and reconstruct the query by replacing the placeholders with their corresponding values. While certainly feasible, I think it's too complicated to bother doing this.

  7. #32
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Thanks. I meant you can do:

    $query = "SELECT * FROM `table`";
    $result = mysqli_query($link, $query);

    if(!$result) {

    log_query($query); // This just writes $query to a text file

    }

    My point was if you using prepared statements and aren't storing the query as a string as above, can you just as easily log the error?

  8. #33
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DrQuincy View Post
    Thanks. I meant you can do:

    $query = "SELECT * FROM `table`";
    $result = mysqli_query($link, $query);

    if(!$result) {

    log_query($query); // This just writes $query to a text file

    }

    My point was if you using prepared statements and aren't storing the query as a string as above, can you just as easily log the error?
    Sure you can, the difference is you will want to catch errors in mysqli_stmt_prepare() and mysqli_stmt_execute() instead of query(). You would then log what you have passed to mysqli_stmt_prepare(). Of course, your logged query will be without data.

  9. #34
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lemon Juice View Post
    Sure you can, the difference is you will want to catch errors in mysqli_stmt_prepare() and mysqli_stmt_execute() instead of query(). You would then log what you have passed to mysqli_stmt_prepare(). Of course, your logged query will be without data.
    That's what I meant so to me that's a bit of a disadvantage.

  10. #35
    ¬.¬ shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Try using:
    http://www.php.net/manual/en/mysqli-...eport-mode.php
    http://www.php.net/manual/en/class.m...-exception.php

    It should give more information when there is an error. Including the failing SQL query. Maybe.

    However, you know. If you don't store the query even if you use the old interface there is no way to get the query unless you have it saved in a variable. But using the exceptions should provide some insight.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  11. #36
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,813
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Lemon Juice View Post
    Oh, now I see where our assumptions are different. When real_escape_string is used like you described above, that is *only* when necessary at all costs, then yes, it can become harder to spot security holes.
    No - even if you always use real_escape_string it is still not as secure as using prepare/bind.

    As an analogy consider the query as a table and the sql as ribbons with the data as presents your visitors place on the table to be wrapped by the ribbons. Escaping puts the presents in bags so that any presents that are ribbons (and a ribbon is a perfectly valid present) don't get mixed up with the ones on the table. The person attempting sql injection is trying to get a ribbon onto the table without it ending up in a bag - their ribbon is not intended as a present. With your suggestion of escaping everything all the tools, toys, furniture, used chewing gum, dead mice etc also get put in bags even though there's no way they can be confused with ribbons just to ensure that you can tell the difference between the ribbons you put on the table and those put there by your visitors as presents and those put there as an injection attempt. It doesn't mean that the bags don't have holes in them or some other way for ribbons to get out of the bags even if you bag everything. It also doesn't mean that all the ribbons in the bags are injection attempts - depending on what you actually accept as valid data.

    Prepare/bind is two tables where you put all the ribbons on one and lock it away in the back room and put the second table where your visitors can put the presents. Now it doesn't matter whether you put anything in bags or not because they are not even in the same room as the ribbons you need to keep separate.

    In the first case a successful sql injection simply requires that the ribbon somehow not end up in a bag or somehow gets out of the bag - unlikely but at least possible for the person placing the ribbon to achieve. In the second case no matter what they do they can't possibly get the ribbon onto the table with the other ribbons because they don't have any access whatever to that table.

    That's the difference between escaping data in the hope that it doesn't get confused with code and keeping the two completely separate so that they can't possibly get confused.

    Now if you add an extra step at the start to validate the data then the fields can't contain junk - if John wants tools as presents then you validate his presents and reject all the toys, furniture, ribbons, dead mice etc. and the bagging of his presents just uses up extra bags without achieving anything else, if Jane only wants ribbons and hairclips then everything except those can be rejected as presents for her but for her presents if you only use one table then the bagging/escaping is essential as her presents can be confused with the ribbons on the table (sql). If you use two separate tables (prepare/bind) then her presents are still on the data table not the sql table so bagging them also just uses up extra bags.

    Without validation you can not only get ribbons being given as presents for John, you can also get billions of dead mice and used chewing gum as well - ie your database will get filled with meaningless junk.

    Have you tested every possible character combination via real_escape_string to check that there are no possible combinations that would allow sql intended to be stored as data to be processed instead (that is that there is no possibility whatsoever that the bags might have a hole in them to allow a carefully designed ribbon to escape onto the table)?
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  12. #37
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,268
    Mentioned
    18 Post(s)
    Tagged
    0 Thread(s)
    This might be one of the more confusing analogies I've ever read.

    I'm a guy who favors prepared statements, but I think I agree with Lemon and the Dr on this one. I agree that it's easier to make mistakes with escaping than with preparing, but I also agree that if both are used properly, then both are equally secure.
    "First make it work. Then make it better."

  13. #38
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    In the second case no matter what they do they can't possibly get the ribbon onto the table with the other ribbons because they don't have any access whatever to that table.
    Why not? How is that they don't have access to that table? Have you locked part of your code so that you can't get to it anymore? I am the programmer and I am the one who prepares the query, what force will ban me from injecting a potentialy unescaped variable into the statement apart from my own sanity? Nobody forces me to use bind() as I should, it's just that I know that I should be using bind() and try to remember that.

    Quote Originally Posted by felgall View Post
    That's the difference between escaping data in the hope that it doesn't get confused with code and keeping the two completely separate so that they can't possibly get confused.
    I wouldn't say SQL containing data is that confusing. SQL has been designed as human-readable language and having data inside the statements is what makes it easily comprehensible. It's like you wanted someone to bring you dinner and beer you would be making two phone calls - in the first one you say "Dinner and beer", hung up and call again and say "Bring me at 4pm, please".

    Sorry if this analogy doesn't make 100% sense but having read yours about tables, ribbons and dead mice I let my imagination run wild

    Quote Originally Posted by felgall View Post
    Now if you add an extra step at the start to validate the data then the fields can't contain junk
    I wouldn't mix validation into the whole thing as I think it should be kept separate and totally independend from both binding and escaping.

    Quote Originally Posted by felgall View Post
    If you use two separate tables (prepare/bind) then her presents are still on the data table not the sql table so bagging them also just uses up extra bags.
    The extra bags are completely free (or the cost is negligible), 100% eco-friendly and disappear after usage without leaving any waste, so I really couldn't care less

    Quote Originally Posted by felgall View Post
    Have you tested every possible character combination via real_escape_string to check that there are no possible combinations that would allow sql intended to be stored as data to be processed instead (that is that there is no possibility whatsoever that the bags might have a hole in them to allow a carefully designed ribbon to escape onto the table)?
    Why would I have to? Escaping is a pretty simple algorythm and I trust that whoever wrote the native function implemented it well and that is enough. Let's not go paranoid here. By the same analogy I could ask you - have you tested adding every possible combination of numbers to check there are no errors in your programming language + operator? If not, then your application is insecure because you may never know how it will behave when faulty addition breaks computing a critical hash!

    I think I have no more arguments in this case anymore so I'll simply agree to disagree!

  14. #39
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    This might be one of the more confusing analogies I've ever read.
    Haha, this wasn't easy but after reading it for a second time I think I grasped the gist more or less

    Quote Originally Posted by Jeff Mott View Post
    I'm a guy who favors prepared statements, but I think I agree with Lemon and the Dr on this one. I agree that it's easier to make mistakes with escaping than with preparing, but I also agree that if both are used properly, then both are equally secure.
    Exactly, it's a little bit easier to keep good security discipline when coding binding than escaping but I don't think the difference is big. It's more because of some of the trade-offs of prepared statements that I mentioned earlier that I tend to favour escaping.

    But it's all down to personal preference. Some people prefer the idea of wrapping up all presents in a paper bag, putting them on table and then getting two guys move the whole table with presents from one room to another, whereas some prefer to mark the spots on the table where each present should be placed and then getting the guys move the table first and come back and get the presents separately and leaving it up to a lady in the other room to place the presents in their proper places marked on the table .

  15. #40
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,813
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Lemon Juice View Post
    Why not? How is that they don't have access to that table? Have you locked part of your code so that you can't get to it anymore?
    No you haven't locked it so you can't get to it - you have locked it so that your visitors can't get to it.

    The prepare statement does not contain ANY variables - it just contains SQL - therefor no matter what your visitor manages to put into any variable in your code it cannot get run as SQL because the only SQL that gets run is in the prepare statement and there are no variables there.

    One last try to explain this. Let's look at the following code which uses prepare/bind to update a database table.

    Code:
    if ($p = $db->prepare('UPDATE memaddr SET missing=?, mail_centre=?, address=?, suburb=?, state=?, postcode=?, country=?, phone=? WHERE member_number=? AND from_date=?')) {
       $p->bind_param('ssssssssss',$missing,$mail_centre,$address,$suburb,$state,$postcode,$country,$phone,$mn,$date);
       $p->execute();
    Now imagine that there is no validation at all and no escaping of any data prior to that code running.

    Now tell me how anything can be supplied in any of the variables that will change the SQL in the prepare statement given that there are no variables in the prepare statement? All the SQL that gets run by the execute call is in the prepare statement so it doesn't matter what the variables in the bind statement contain as whatever is passed there is data - injection into the prepare statement is impossible because it doesn't contain any variables.

    A query call would be just as secure provided that you don't include any variables in the SQL. Of course that would mean that it can only do what you have predefined it to do and it cannot use any data supplied by visitors as they'd need to be supplied in variables.

    Once you include variables in the same call as the SQL you introduce the possibility of that variable changing the SQL. While you escape the variables to try to prevent that confusion they are still in the same statement and so there is at least a possibility of finding a value that will get past the escape call and modify the SQL. While the escape function is supposed to prevent injection how do you know that it doesn't contain a security hole that will allow injection to occur? You are trusting someone else's code doesn't have a security hole - which is potentially less secure than using an alternative that doesn't depend on code not having security holes.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  16. #41
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,268
    Mentioned
    18 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    Once you include variables in the same call as the SQL you introduce the possibility of that variable changing the SQL. While you escape the variables to try to prevent that confusion they are still in the same statement and so there is at least a possibility of finding a value that will get past the escape call and modify the SQL.
    In sounds like we're debating theory vs practice. In theory, a user might, some how, some way, find a way to leak through the escaping. But in practice, I don't see any evidence of that. I think we have our bases covered.

    Quote Originally Posted by felgall View Post
    While the escape function is supposed to prevent injection how do you know that it doesn't contain a security hole that will allow injection to occur?
    This is getting a bit paranoid. We could apply this logic to everything in PHP. How do we know that function X was implemented correctly?
    "First make it work. Then make it better."

  17. #42
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,813
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    In sounds like we're debating theory vs practice. In theory, a user might, some how, some way, find a way to leak through the escaping. But in practice, I don't see any evidence of that. I think we have our bases covered.
    In that case why has PHP taken the first of two steps to prevent you from being able to use calls that jumble the code and data together by removing the interface that doesn't allow you to keep them separate?

    Why jumble code and data together when you don't have to? Where there is a language construct that keeps them separate you completely eliminate all possibility of data being processed as code if you use that construct.

    In practice jumbling them together is a mess and escaping the data is a patch. Keeping them separate is tidier and removes reliance on you remembering to escape data in order to patch it to work with the mess.

    PHP is removing the mysql_ interface because it doesn't support ways to keep the code and data separate. Both of the replacements - mysqli_ and PDO - recommend using prepare/bind rather than query - presumably the option to use query and to jumble the code and data together will be removed in a later version and is only supported now to make it easier to migrate from a jumbled mess of code and data to keeping them completely separate by doing it as two steps instead of all at once.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  18. #43
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    In practice jumbling them together is a mess and escaping the data is a patch. Keeping them separate is tidier and removes reliance on you remembering to escape data in order to patch it to work with the mess.
    Each to their own, to me a query with data looks cleaner than a query with placeholders plus a set of data as a separate entity. And I don't think escaping was ever done as a patch - from what I know prepared statements were created to improve the performance of repeated queries with different sets of data.

    Quote Originally Posted by felgall View Post
    presumably the option to use query and to jumble the code and data together will be removed in a later version
    If I were to make the prediction this will never happen! Don't forget SQL is a language and has to be easy to be understood by humans. It's as if you said that future versions of PHP won't allow to jumble data (literal string, numbers, etc.) with the code (statements, constructs, functions, etc.) and instead each script will require two separate files - one with the code (and placeholders) and the other with data!

  19. #44
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,268
    Mentioned
    18 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by felgall View Post
    Why jumble code and data together when you don't have to? Where there is a language construct that keeps them separate you completely eliminate all possibility of data being processed as code if you use that construct.
    I actually agree with you here. As I said, I favor prepared statements. The only point that people were taking issue with was the claim that escaping is insecure, based on some hypothetical vulnerability that doesn't seem to exist in practice. I agree that prepared statements seem tidier, but I don't agree that escaping is insecure.
    "First make it work. Then make it better."

  20. #45
    Programming Since 1978 silver trophybronze trophy felgall's Avatar
    Join Date
    Sep 2005
    Location
    Sydney, NSW, Australia
    Posts
    16,813
    Mentioned
    25 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    I actually agree with you here. As I said, I favor prepared statements. The only point that people were taking issue with was the claim that escaping is insecure, based on some hypothetical vulnerability that doesn't seem to exist in practice. I agree that prepared statements seem tidier, but I don't agree that escaping is insecure.
    I never said that escaping is insecure. I said that using an approach that requires escaping is less secure than one that is inherently secure.

    With prepare/bind you do not need to look beyond the prepare statement to know that it is secure. With an approach that requires escaping you need to look beyond the database call and into the PHP code to know whether the call is secure or not - that has the potential to be less secure simply because there is more code you need to look at to check security.

    PHP has always been able to produce completely secure code but most people using it do not learn about security when they learn how to write code and so have in many cases used code in ways that have the potential to be less secure. As a result those responsible for PHP are gradually removing those ways of coding that have the greatest potential for being misused to produce less secure - hence register globals got turned off by default and then completely deleted and similarly the mysql_ interface is being removed. PHP is being made into a more secure language by removing the ways that insecure code could be produced.

    Jumbling code and data together is less secure than keeping them separate BECAUSE it requires that any data that could be misread as code be escaped. Using real_escape_string is less secure because you might forget to apply it to a field that needs it. Yes there is a really slight possibility of it containing a security hole that we can disregard as if such a hole ever gets found then it will get fixed for us - leaving it off a field by accident is a far higher possibility and that's probably the one that makes the biggest difference - particularly in a medium to large application where you can't just apply it to all database fields and slow the processing down for numbers and dates and other things that cannot possibly contain anything that could possibly be mistaken for code - where the person responsible for optimising the code will strip out the escaping of those fields where escaping is not needed so as to speed up the processing by hopefully enough to make the application useable.

    Escaping everything whether it needs to be escaped or not is only really suitable for tiny applications with only a few hundred users. Once you have several million users all trying to make database calls at the same time you will need to have the processing run as efficiently as possible and unnecessary calls such as escaping dates will get stripped out.
    Stephen J Chapman

    javascriptexample.net, Book Reviews, follow me on Twitter
    HTML Help, CSS Help, JavaScript Help, PHP/mySQL Help, blog
    <input name="html5" type="text" required pattern="^$">

  21. #46
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quick aside. With prepared statements:

    1. If you don't validate your data and you pass '345regdfgtr' as an int, does the query just fail?

    2. If you get the type wrong, (e.g. s for a double) does the query just fail too? (I.e. no injection)

    I am going to try this for myself when I get the chance but thought I'd ask.

    Regarding the main discussion, I agree this is a matter of preference and perhaps you could use standard queries and prepareds side-by-side and get the best of both world (i.e. standard querys when no user input and you're only running it once, prepared for everything else).

    I for one put all user input through a cleaning function and have to explicitly pass false to not escape it. There's still a chance of injection through my error, for sure, but then every line of code I write could contain an error.

  22. #47
    SitePoint Guru bronze trophy
    Join Date
    Dec 2003
    Location
    Poland
    Posts
    930
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DrQuincy View Post
    Quick aside. With prepared statements:

    1. If you don't validate your data and you pass '345regdfgtr' as an int, does the query just fail?
    No, it will not fail but the value will be converted to a number based on some "best guess" mechanism. You'd have to test it yourself because I can't remember the exact behaviour when I did such tests some time ago. Most probably your sample string will be converted to number 345, and if the string begins with a non-digit then it will be converted to 0.

    Quote Originally Posted by DrQuincy View Post
    2. If you get the type wrong, (e.g. s for a double) does the query just fail too? (I.e. no injection)
    No, the query will never fail and you are always safe from injections. The only bad thing that can happen is that the value might get converted to a different one. Generally, you could get away with always setting the type to string since this would be equivalent to values in single quotes in SQL and mysql permits them. But as I said earlier, there can be some edge cases like very big numbers when the type you set might influence the value used by the query so to be 100% safe I'd set the types properly.

    Quote Originally Posted by DrQuincy View Post
    I for one put all user input through a cleaning function and have to explicitly pass false to not escape it.
    Do you mean you put every input variable through real_escape_string? While filtering and sanitizing data is okay, I think using real_escape_string on all input by default is not a good idea because I think responsibility for escaping data lies in the code that actually constructs the query, not in the code which receives the input and validates it. If you have all input data escaped from the start then you can only use the data in an sql query. Suppose later you want to add some more code that will additionally send some of the input data via email or store in an XML file - then you'd have to unescape the data to get rid of all escape characters that are just junk for the other purposes. This is one of the reason why magic_quotes were removed from PHP, which essentially did the same thing - they auto-escaped all input data.

  23. #48
    SitePoint Evangelist
    Join Date
    May 2006
    Posts
    436
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lemon Juice View Post
    No, it will not fail but the value will be converted to a number based on some "best guess" mechanism. You'd have to test it yourself because I can't remember the exact behaviour when I did such tests some time ago. Most probably your sample string will be converted to number 345, and if the string begins with a non-digit then it will be converted to 0.


    No, the query will never fail and you are always safe from injections. The only bad thing that can happen is that the value might get converted to a different one. Generally, you could get away with always setting the type to string since this would be equivalent to values in single quotes in SQL and mysql permits them. But as I said earlier, there can be some edge cases like very big numbers when the type you set might influence the value used by the query so to be 100% safe I'd set the types properly.



    Do you mean you put every input variable through real_escape_string? While filtering and sanitizing data is okay, I think using real_escape_string on all input by default is not a good idea because I think responsibility for escaping data lies in the code that actually constructs the query, not in the code which receives the input and validates it. If you have all input data escaped from the start then you can only use the data in an sql query. Suppose later you want to add some more code that will additionally send some of the input data via email or store in an XML file - then you'd have to unescape the data to get rid of all escape characters that are just junk for the other purposes. This is one of the reason why magic_quotes were removed from PHP, which essentially did the same thing - they auto-escaped all input data.
    Only if they're strings and going into the database. If they're going into an email, for example, I would set escaping to false to avoid the extra characters. I understand what you are saying and fully agree.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •