How do I secure?

[jrws]

Hey guys,

Currently I am using a protect function:

function clean($string)
{
    if (get_magic_quotes_gpc())
    {
        $string = stripslashes($string);
    } elseif (!get_magic_quotes_gpc())
    {
        $string = addslashes(trim($string));
    }
    $string = trim($string);
    //$string = escapeshellcmd($string);//Uncomment if the website uploading to allows this.
    $string = mysql_real_escape_string($string);
    $string = stripslashes(strip_tags(htmlspecialchars($string)));
    return $string;
}

This is used to sanitize the user input, however I recently downloaded a program for firefox that checks against sql attacks, and I have seven possible attacks that can happen:

SQL Injection String Test Results
username
Submitted Form State:

    * password:
    * login: Login
    * remember_me: Remember Me

Results:
Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1 UNION ALL SELECT 1,2,3,4,5,6,name FROM sysObjects WHERE xtype = 'U' --

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1 AND ASCII(LOWER(SUBSTRING((SELECT TOP 1 name FROM sysobjects WHERE xtype='U'), 1, 1))) > 116

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: '; DESC users; --

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1' AND 1=(SELECT COUNT(*) FROM tablenames); --

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1'1

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1 AND USER_NAME() = 'dbo'

Error string found: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use'
Tested value: 1'1

So can I secure against these? Or are they nothing to worry about?

[PFMaBiSmAd]

You do need to worry, because your clean() function is running stripslashes() on the data right before it is returned, so most of the protection that the code is deigned to add is removed and has no effect.

 

The bad news -

 

When magic_quotes_gpc are not on, don't use addslashes(). That with the use of mysql_real_escape_string() would cause double escaped data.

 

The characters that trim() removes from the start and end of a string are not a direct security issue and using trim() does not add any protection. The same characters could be within the string and could still break the query. mysql_real_escape_string protects against the use of these characters no matter where they appear in a string. Trim() should only be used if your application needs to trim characters from the start and end of a string and not for any protection reasons.

 

htmlspecialchars() converts the special characters that define what a tag is into HTML entities and running strip_tags() after that point has no effect because there are no literal tag characters remaining in the code to strip. If you want to remove tags, do it before htmlspecialchars().

 

htmlspecialchars() by default (without the second parameter) leaves single quotes alone, so if you were outputting the results to the browser, it could have single quotes in it that could be used for some unintended purpose.

 

htmlspecialchars() does not convert all the special characters. Use  htmlentities() instead.

 

Some of the tests used numeric data. Your clean function is deigned for string data being put into strings (surrounded by single-quotes in the query.) For numeric data (not surrounded by single-quotes in the query) you need to validate that the data is a number or simply cast it as a number to prevent sql injection in fields that are numeric.

 

For your clean function (for string data only), the following would be an improvement -

 

function clean($string)
{
    if (get_magic_quotes_gpc())
    {
        $string = stripslashes($string); // remove slashes if magic_quotes_gpc added slashes
    }
    $string = strip_tags($string); // strip HTML/php tags
    $string = htmlentities($string, ENT_QUOTES); // convert remaining HTML special characters to entities
    $string = mysql_real_escape_string($string); // escape remaining special characters
    return $string;
}

[jrws]

Thanks for that, what about numerical data? Something like is_numeric($code)?