SQL Injection and mysql_real_escape_string - Page 2

delayedinsanity · 05-02-2008, 11:30 PM

Semi-off-topic, but is there a way to typecast an objects variables previous to them being set? I was hoping I could do;

PHP Code:


			
public (string)$m_szString;
public (int)$m_iInteger;

private (array)$m_aArray;

at the top of my classes, but it turns out that's a no-no.
-m

Village Idiot · 05-03-2008, 12:15 AM

Quote:

Originally Posted by wGEric

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

PHP Code:


			
'1' !== 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

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

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

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

Salathe · 05-03-2008, 12:56 AM

Quote:

Originally Posted by delayedinsanity

Semi-off-topic, but is there a way to typecast an objects variables previous to them being set? I was hoping I could do;

PHP Code:


			
public (string)$m_szString;

public (int)$m_iInteger;



private (array)$m_aArray;

at the top of my classes, but it turns out that's a no-no.
-m

No, there isn't. PHP is a dynamically typed language and any initial casting, even if it were applied, could be overridden at any point (hence, dynamic).

Harking back to the topic at hand somewhat, please remember the name of the function being used to escape your input: mysql_escape_string or MySQL Escape String. Escape string. Ok perhaps that came off a little strong but the point is there to be made that that function will only escape string values where expected input and output are strings; not integers, booleans or other types.

delayedinsanity · 05-03-2008, 01:09 AM

Quote:

No, there isn't. PHP is a dynamically typed language and any initial casting, even if it were applied, could be overridden at any point (hence, dynamic).

As I'm aware, and have been thankful for a lot at various times in my bumbling efforts to learn. Despite being dynamically typed, I just figured there may not be any harm in suggesting to the compiler that I was hoping maybe it would be of a specific type at all given times.

Off I go...
-m

Village Idiot · 05-03-2008, 03:07 AM

Quote:

Originally Posted by Salathe

Harking back to the topic at hand somewhat, please remember the name of the function being used to escape your input: mysql_escape_string or MySQL Escape String. Escape string. Ok perhaps that came off a little strong but the point is there to be made that that function will only escape string values where expected input and output are strings; not integers, booleans or other types.

If that was directed at me, I would suggest reading my posts. Also, use mysql_real_escape_string, mysql_escape_string is depreciated since 4.3.0.
PHP: mysql_escape_string - Manual

Salathe · 05-03-2008, 03:37 AM

It wasn't directed at you, because I did read your posts and understood what's being said. No need to be defensive (that's what your reply came across as). I also meant to reference mysql_real_escape_string but early morning brainfarts stole the 'real' from my fingers. My point was simply that folks seems to ignore the 'string' portion of the function name and try to escape everything with it.

Highway of Life · 05-03-2008, 05:17 AM

PHP is a language that is very open to different types of coding.
Many good, many bad. There are always multiple ways of doing something in PHP, that’s one reason it is such a great language.
Everyone has their own way of doing something. The most common problem, however, is lack of proper knowledge of the language. Take someone who has worked on Java or C for 20 years and put them in front of PHP, they will learn the language quickly, but they will not necessarily pick up the true and correct methods and concepts of the language. Amazingly, few do.

In working with many PHP programmers as an instructor, I’ve seen coders from every level, starting from novice to advanced.
From inexperienced beginners to 20-30 years of experience programming C-based languages.
It is an easy language to learn, but a difficult language to master -- properly.
But even as easy as the language is, as popular as the language is, as many people program in it, few code correctly and securely as the language was intended to be programmed in. -- As an instructor, I regularly see code that is vulnerable in one way or another, about 1 in every 3 scripts has a severe vulnerability of some kind (75% of the time, it is an SQL Injection and usually due to poor or no sanitisation methods).
These are mostly due to not understanding typecasting properly, not using it, or not understanding the concepts behind proper coding in PHP, including coding PHP securely and the way it was intended to be used.

The reason that there were 20,000 sites hacked between January and April of 2008 was due to SQL Injections in web applications that hackers exploited to insert code that would inject iframes into every web-based dynamic (ASP or PHP) file on the website.
Some 200,000 plus webpages contained these iframes.
So how do people address this? How do you, as a site owner and coder, secure your site or ensure that the applications or scripts that you use or code are going to be secure and free from exploits?
You code in the way that PHP was intended, and the way that PHP was intended to work with databases.
Sure, you can program PHP in the way that you want to, you can use shortcuts, or do things your way because you feel like it, or you can program in a way that is correct and secure.

One of the most important concepts that I’ve had to drill into the heads of my students has been proper sanitisation of variables AND type casting.
Type casting is equally as important as sanitisation of strings.
PHP is built to be a dynamic language where you can dynamically assign an assortment of type definitions to a variable, it does not require or support explicit type definitions, this means that it is set by the content that is assigned to the variable.

If your Database field is intended to contain a float, you, the programmer must ensure that the data you are placing into that field is a float. If the field is an integer, you must ensure that the variable is an integer. If it is meant to contain a string, you must ensure that the variable type is going to be a string.
User data can be entered as a string, integer, float, or an array.
You cannot insert an array into a string field, a string field into an integer field, or float into an integer field (for example).

When you create a database, you define the fields to be exact database types, exact numeric data types such as Integer, decimal and numeric, approximate numeric data types such as float, real or double, a bit data type (as of MySQL 5), date and time data types, string data types such as char, varchar, text, binary (similar to char, but stores binary byte strings), blob and enum.

When creating a database, you do not create each field to be a string, you create the field with the data type of the information it will be storing -- I hope I don’t have to go into examples about *why* you must do this with databases --.
You then assign data to these database tables through your database interface, most commonly the built-in mysql functions in PHP.
When inserting data, they are meant to contain the data type that you have assigned to the field. If you created an numeric data type field, you need to ensure that the variable is an integer or float (float, double or real) as necessary.
If you do not, you introduce many potential and guaranteed problems into both your code and your database, database errors notwithstanding.

If you insert string data types, you need to ensure that the data that you are inputting is not a resource or array. -- Integer data types are allowed in string data type fields.
That means that even when pulling data, such as $_POST['my_variable'], you cannot just input this straight into your database, obviously, you need to sanitise the variable.
Your method of sanitisation does depend on what data type the field is that you will be inserting it into.
If it is int, force var-type int, if it is a float, you need to force it to be a float or double. Remember, PHP is a dynamic language, so it will change the var-type easily.
The variable may be an array, which you will need to deal with if you are attempting to insert the data into a string data type field.

All these must be must be considered if one is to create a script that is completely secure from Injection. -- If you want to ensure you are secure, create your code the way PHP intended for it to be created. Use proper type-casting.
It will save you many headaches down the road, it is well worth the research anyone would bother to spend on data types and type casting in PHP and your database.

People take database manipulation and interaction much too lightly, and it shows... 20,000 sites hacked in 4 months is too many. Certainly unnecessary and could have been avoided if the programmers had bothered to become familiar with correct sanitisation methods and typecasting.

If you are a user who is unsure about your script, you are unsure if you are using proper sanitisation and type casting methods, talk to programmers who you know are experienced in the language you are working with.
Do some research, on type casting and proper usage of the mysql_real_escape_string() function. If you are not using MySQL, check your DBMS type to see what function it uses to properly sanitise strings.
And last but not least, read PHP: Type Juggling - Manual on how to properly use typecasting in PHP.
And use Google to research the topic in question, a little bit of research will go a long way to providing you with the necessary knowledge to build secure and stable scripts for your site or your clients.

- Highway of Life

Village Idiot · 05-03-2008, 01:03 PM

A nicely written post, but still a straw man argument. The only reasons you give for security typecasting is that it is a basic method everyone should know. Even though I gave example after example of how my method is just as secure (and less susceptible to human error), you just say its against basic knowledge.

Quote:

People take database manipulation and interaction much too lightly, and it shows... 20,000 sites hacked in 4 months is too many. Certainly unnecessary and could have been avoided if the programmers had bothered to become familiar with correct sanitisation methods and typecasting.

You blame all these on not typecasting?

Don't expect a reply unless you start presenting logical arguments on why my method is insecure (like you claim it is). The best argument you put forward is for inserting something, which is better solved by checking types and raising a flag if not that type.

I am not a student of yours, I wont get an F for calling you wrong, a higher standard of proof is required than you present in class. Moreso because you are talking to an experienced PHP programmer who has a tried true method.

People take database manipulation and interaction much too lightly, and it shows... 20,000 sites hacked in 4 months is too many. Certainly unnecessary and could have been avoided if the programmers had bothered to become familiar with correct sanitisation methods and typecasting.

Bottom Line
My portfolio site is JustAnotherPortfolio - Index. Come back when you have exploited one of my scripts. If you cant, please stop presenting your opinion as if it where absolute fact.

Highway of Life · 05-03-2008, 06:08 PM

Since your code sample is hidden behind 10 SQL errors, I am unable to look at it.

You seemed to have missed every point in my post.

Village Idiot · 05-03-2008, 07:51 PM

Quote:

Originally Posted by Highway of Life

Since your code sample is hidden behind 10 SQL errors, I am unable to look at it.

You seemed to have missed every point in my post.

I seem to have forgotten to update a password since I changed, my other scripts are not.

You seem to be missing the point of my post, present facts.

delayedinsanity · 05-03-2008, 07:55 PM

I was avoiding this, but c'mon. He's presented a lot more thorough reasoning than you have - to date all you've said, summed up, is "my method works fine. I don't need yours.". Which btw, after looking at your code sample (which I might add is 95% just Smarty, you may want to make a note of that so unsuspecting clients don't assume that it's your work), it seems the only thing you do is check if a variable isn't numeric, and if so, you mysql_real_escape_string() it. If that's the extent of your validation/sanitization and security, that's pretty flimsy.
-m

Village Idiot · 05-03-2008, 08:11 PM

Quote:

Originally Posted by delayedinsanity

I was avoiding this, but c'mon. He's presented a lot more thorough reasoning than you have - to date all you've said, summed up, is "my method works fine. I don't need yours.". Which btw, after looking at your code sample (which I might add is 95% just Smarty, you may want to make a note of that so unsuspecting clients don't assume that it's your work), it seems the only thing you do is check if a variable isn't numeric, and if so, you mysql_real_escape_string() it. If that's the extent of your validation/sanitization and security, that's pretty flimsy.
-m

I have provided far more than that, I have provided my method of validation and clearly walked you though my process. All highway has posted is I am wrong, without a single piece of evidence. Also, that is not all I do, I put everything between single quotes, meaning When this is done with my previous processes, it is just as secure as typecasting! Is anyone even reading my posts? I am not saying there is no use for typecasting, there are specific uses where it may be necessary, but it is not a basic of security type thing. There is a reason highway didn't show me an example of injection on any of my clients sites, because he can't. Am I the only one that finds it funny that you are saying my scripts are not secure, but you cant seem to hack them?

delayedinsanity · 05-03-2008, 08:21 PM

Probably because I'm not a hacker, nor does your sample script even run to make the attempt on, nor do I have the time. However, after looking at your script, with all the single quotes, and the only other method called is mysql_real_escape_string, I'm going to firmly believe that your scripts are entirely and probably very easily hackable.

The first SQL injection attack I ever read about was something akin to: ' username='admin' --, which makes use of the fact that the script is probably using single quotes, so I hardly see how you believe this to be a form of security.
-m

Highway of Life · 05-03-2008, 08:48 PM

Quote:

Originally Posted by Village Idiot

There is a reason highway didn't show me an example of injection on any of my clients sites, because he can't.

You know what, It’s poor sport to attack users when you lose an argument. You don’t need to put words in my mouth.

I could never teach someone who is not willing to learn or improve their knowledge of programming and or security. Someone who is not willing to admit when they are wrong.
Someone who attacks others because he has lost an argument. I see no point in continuing, I said what I came to say, as it was important that people don't get the wrong idea from Village Idiot's post -- or believe that his methods are anywhere near correct. [ Although, I suppose I should have read the username before replying ]

Those who have read this topic, I implore you to do your own research, I think the answers will be quite clear.

Arguing to argue is pointless, I will no longer post in this topic to continue a pointless argument.

Village Idiot · 05-03-2008, 08:54 PM

No, to use straw man arguments and throw opinion around like it is fact is pointless. You outright are calling my method insecure. Now meet my challenge and exploit one of my scripts or come to the truth that my method works.

Village Idiot · 05-03-2008, 09:11 PM

I am having some database troubles, I'll have that fixed tonight if possible. I dont have the time right now.

ps, why did you delete your post?

Village Idiot · 05-03-2008, 09:21 PM

Quote:

Originally Posted by Highway of Life

I said what I came to say, as it was important that people don't get the wrong idea from Village Idiot's post -- or believe that his methods are anywhere near correct.

Then directly, right here, right now, show me what is wrong with my method. I gave my cleaning method a few posts ago. Right here, right now, show me whats wrong or stop spewing shit and pretending you know everything. You will probably just say you have shown it, all you have given is weak speed arguments. No paragraps about how simple knowledge it is, none of that. Just the error in my method. Not that you disagree with it, how it opens a hole in my script. If there really is a opening to SQL injection, I will go to all my clients and fix it. But you will have to demonstrate this, you haven't

delayedinsanity · 05-03-2008, 09:23 PM

Bro, seriously if you're this upset, maybe you outta take a break and just walk away. It's not that important.
-m

Village Idiot · 05-03-2008, 09:30 PM

Quote:

Originally Posted by delayedinsanity

Bro, seriously if you're this upset, maybe you outta take a break and just walk away. It's not that important.
-m

I'm mad because potential clients may read this and get the impression that I do not know how to securely program something. I don't care of someone calls me an idiot, but do something that can hurt my business and it becomes a little more serious.

delayedinsanity · 05-03-2008, 09:50 PM

I'd be more worried about potential clients seeing me lose my temper with somebody who disagreed with my process. I've read other posts from you around here, and I wouldn't call you an idiot, but I've been reading his, and I wouldn't call him one either. There's no reason you both couldn't learn something from eachother.
-m