View Single Post
Old 05-03-2008, 12:15 AM   #22 (permalink)
Village Idiot
Wizard
Top Contributor 
 
Village Idiot's Avatar
 
Join Date: Sep 2007
Posts: 1,299
Thanks: 17
Village Idiot is on a distinguished road
Default

Quote:
Originally Posted by wGEric View Post
So you are saying performance is more important than security?
By no means, but who wouldnt choose the faster just as secure method?


Quote:
Originally Posted by wGEric View Post
PHP Code:
'1' !== 
Which is what you are trying to do unless all of your columns are some sort of string type. That's the downside. You are trying to compare a string with an integer or you're inserting a string into an integer column. That doesn't make sense.[/php]
I do it in every script I make, if var = 1, '$var' can be compared to in integer in mysql. Mysql is indifferent on this. If you want to refer to the official mysql manual, they will tell you that the easiest way is to wrap all your values in single quotes.



Quote:
Originally Posted by wGEric View Post
That's what mysql_real_escape_string and other methods for cleaning strings are for. They aren't for numbers. That's why it has string in the name.[/php]
You can still inject commands into an escaped string.


Quote:
Originally Posted by wGEric View Post
Wait, so you are using a string that hasn't been sanitized? Or do you type cast it as an integer before using it? If the later, why bother making it a string in the query. You already know it is secure.
I sanitize my strings, but if they arent put between single quotes, commands can still be injected in them. Mysql_real_escape_string() only escapes a few characters. Why make it a string in the query? Habit, you don't want to accidentally leave a string non wrapped. I don't want to have to double check everything, so why not just put everything in quotes?

Harder method: Typecast every int that is going into the database, never forget even one (or mistake a string for being one) because if it is a string you will be open to injection.

Easier method: Put every variable though the escape function, the function will escape anything that is not an integer. Just put everything in single quotes. Do this uniformly for everything and you will be safe

I have included the object I use for data.


Quote:
Originally Posted by wGEric View Post
By not typecasting you aren't forcing a variable to be what you expect and want it to be. If you want an integer, make it an integer so you aren't dealing with strings which can be exploited. It's so much easier to secure an integer than a string
Typecasting only makes sure an int is an int, it doesn't make sure its what you want it. You don't need to typecast as long as you clean everything. Just have it check if its an int and don't escape it if so. By doing this you reduce any possible error of forgetting a step. The object I use replaces $_GET, ect. It gets that and cleans it, removing any possibility that I would forget to escape it unless I call it incorrectly (which has by habit become extremely unlikely).

I find it easier not to typecast, there really is no security downside to it.


Here is the object I use to clean data as it comes in.
PHP Code:
class input
{
    function 
sql_safe($value
    {
        
// Stripslashes
        
if (get_magic_quotes_gpc()) 
        {
            
$value stripslashes($value);
        }

        
// Clean if not integer
        
if (!is_numeric($value) || $value[0] == '0')
        {
            
$value mysql_real_escape_string($value);
        }

        return 
$value;
    }

    function 
get($var)
    {
        
$var $_GET[$var];
        
$var $this->sql_safe($var);
        return 
$var;
    }

    function 
post($var)
    {
        
$var $_POST[$var];
        
$var $this->sql_safe($var);
        return 
$var;
    }

    function 
cookie($var)
    {
        
$var $_COOKIE[$var];
        
$var $this->sql_safe($var);
        return 
$var;
    }

This way, there is no room for error unless I get a variable via $_GET. I just get all my data via this object and it cleans it for me. No worries and less room for human error.

I seem to be repeating myself a lot here, so here is the bottom line for those who don't read entire posts:
Typecasting is a fine method that, with the other proper measures, will get the job done perfectly. But in my opinion, it leaves too much room for human error. The method I have put forward, when used with the other proper measures, will get the job done just as well as the typecasting method. It boils down to opinion and personal style, there is no right way to do this (although there are wrong ways).
__________________

Village Idiot is offline  
Reply With Quote