PHP Security

[Trigs]

I use this to clean all user input

function clean($str) {
 $str = trim($str);
 if(!get_magic_quotes_gpc()) {
     $str = addslashes($str);
    }
 $str = strip_tags(htmlspecialchars($str));
 return $str; 
} 

What's not needed and what can I add to increase it's effectiveness?

[Apolva]

There are flaws with addslashes which can allow injection using other character sets, therefore it's better to use mysql_real_escape_string().

Here's a more compact version:

function clean($str) {
     if(get_magic_quotes_gpc()) $str=stripslashes($str);
     return strip_tags(htmlspecialchars(mysql_real_escape_string(trim($str))));
} 

That would sanitize a string, but remember there are many other aspects to server-side security you'll need to take account for

[Trigs]

What else can I add?

[HotelUser]

technically speaking why can't one simply use str_replace to phrase the single quotes? I have always wondered this, although I use these two simple classes:

function clean($u)
{
    $u = stripslashes($u);
    return $u;
}
function cleanArray(&$a)
{
    foreach($a as $id => $value)
        $a[$id] = clean($value);
} 

[Source]

In my scripts I often only use strip_tags, htmlentities and where applicable trim and mysql_real_escape_string. So Apolva's sample will more than cover you in most situations.

Sometimes, if you are working with POST data a lot it might be an idea to make a global filter function that you run before using $_POST, $_GET (or other globals).

<?php

function filterGlobals()
{
    foreach( $_POST as $key => $value )
    {
        $_POST[$key] = htmlentities( strip_tags( mysql_real_escape_string( trim( $value ) ) ) );
    }
}

?>

Of course you would make a function just the use array walk and walk the arrays through your cleaning function. That was just some code to give you the general idea.

The main bulk of security comes from common sense and not cutting corners. Make sure file upload are what they say they are etc.

[Apolva]

Few other suggestions:

• Ensure you have register_globals turned off. This "feature" opens up means for a myriad of other exploits, eg. not clearing a variable first means it may contain user input, also global overwriting attacks.

• Suppress or disable php errors - they reveal your www directory location, which is helpful for blind attacks (and sometimes reveal how your script works, eg. "Failed to open file: passwords.txt" - although you shouldn't use a .txt for this anyway.).

• If you have a user system which a lot of people use, pass along a random "token" in the URL for every page request after log in - if the request doesn't have the correct token, end execution immediately (die()). This is to eliminate XSRF.

• Require passwords for things - if you're changing a password, require the old one, perhaps also password protect more "risky" controls in a control panel.

• Be careful when dealing with files names. For example, ?page=../../privatefile used on include($_GET['privatefile'].".php"); would allow you to execute or view other unintended files (combined with register_globals this could be lethal).

• File uploads need proper filtering to determine if they're safe or not. For example, don't allow "file.php" to be uploaded as-is, else the user can then run it on your server. Instead rename to a random 20 or so letter file name in a separate directory and use a database to remember the original name, then you can send out a http file attachment header with the original name.

[Trigs]

Thanks. I already all of what you posted other than the token thing. I'll look into it.

+rep.