Page 1 of 1

Cleaning all $_POST or $_GET - error in code

Posted: Sun Jan 30, 2011 8:59 am
by sockpuppet
Hello, I'm trying to make a function that cleans all the $_POST or $_GET varaibles and puts them into a nice $CleanVars array for me. But I'm getting the error: Parse error: syntax error, unexpected ')', expecting T_PAAMAYIM_NEKUDOTAYIM in /site_functions/main.functions.php on line 136

Line 136 is "foreach($_POST as $key=>value)" and it seems the error is about missing double dots? But I'm lost as to how to fix it.

Any ideas, or does anyone have a decent script that cleans all the $_GET, $_POST vars to prevent XSS and sql attacks?

Richard

Code: Select all

function ba_clean_all_vars($type) { 

	if($type=="POST") {
		foreach($_POST as $key=>value) {
			$notclean = array_map("stripslashes",$value);
			$notclean = array_map("mysql_real_escape_string",$notclean);
		
			$nowclean[$key] = $notclean;
		}
	} else {
		foreach($_GET as $key=>value) {
			$notclean = array_map("stripslashes",$value);
			$notclean = array_map("mysql_real_escape_string",$notclean);
		
			$nowclean[$key] = $notclean;
		}	
	}

	return $nowclean;
}

Code: Select all

$CleanVars = ba_clean_all_vars('POST');

Re: Cleaning all $_POST or $_GET - error in code

Posted: Sun Jan 30, 2011 9:23 am
by kr1pt

Code: Select all

function ba_clean_all_vars($type)
{

    if ($type == "POST")
    {
        foreach ($_POST as $key => $value)
        {
            $notclean = array_map("stripslashes", $value);
            $notclean = array_map("mysql_real_escape_string", $notclean);

            $nowclean[$key] = $notclean;
        }
    }
    else
    {
        foreach ($_GET as $key => $value)
        {
            $notclean = array_map("stripslashes", $value);
            $notclean = array_map("mysql_real_escape_string", $notclean);

            $nowclean[$key] = $notclean;
        }
    }

    return $nowclean;
}

Re: Cleaning all $_POST or $_GET - error in code

Posted: Sun Jan 30, 2011 10:14 am
by social_experiment
sockpuppet wrote:Any ideas, or does anyone have a decent script that cleans all the $_GET, $_POST vars to prevent XSS and sql attacks?
This script seems to handle it nicely, to stop injection attacks there is already a mysql_real_escape_string() function in place. To prevent XSS you display the values using htmlentities(). This will be when you output your data back to the browser.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Mon Jan 31, 2011 8:49 am
by John Cartwright
If you are going to blindly apply mysql_real_escape_string() to the input, you must be mindful that it will modify the existing string if it has to escape any characters.

Code: Select all

$username = "John O'Brian";

if (mysql_real_escape_string($username) != $username) {
   echo 'we have a modified username!';
}
A better approach would be specify the fields you are expecting, and validate/filter against the expected format. You generally only want to apply your escaping when dealing with queries.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Mon Jan 31, 2011 10:04 am
by sockpuppet
Thanks all, the mysql_real_escape_string() has been removed as it was just there for testing purposes. I've got a db wrapper that escapes each variables on query so its not needed twice. This is more about preventing XSS attacks using the variables.

I might give it a second array input with

fieldname => expected_format

To make cleaning easier/more secure.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Tue Feb 01, 2011 12:42 am
by wastedgod
I know your question is answered but wanted to suggest checking out php's filter_var function. Does a pretty good job of data type and allowed char filtering.

Also for XSS prevention I try to use prepared statements when ever possible.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Tue Feb 01, 2011 5:25 am
by VladSun
wastedgod wrote:Also for XSS prevention I try to use prepared statements when ever possible.
:dubious:
XSS and SQL injections are very different things.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Tue Feb 01, 2011 9:38 am
by wastedgod
XSS and SQL injections are very different things.
Correct, my fault for confusing the two.

Re: Cleaning all $_POST or $_GET - error in code

Posted: Tue Feb 01, 2011 6:44 pm
by dafydd
Here's a side thought for you. If you ever code your html to have a <select> that allows for multiple values, your $value variable will start out as just the last value selected.

Code: Select all

<select name="available" multiple="multiple" size="7">
  <option value="0">Sunday</option>
  <option value="1">Monday</option>
  <option value="2">Tuesday</option>
  <option value="3">Wednesday</option>
  <option value="4">Thursday</option>
  <option value="5">Friday</option>
  <option value="6">Saturday</option>
</select>
If your user picks Thursday, Friday, and Saturday, the reply will look something like

Code: Select all

available=6&available=4&available=5
and your cleaner code will only recognize whichever value came last. The first trick around this is to cheat with your html code.

Code: Select all

<select name="available[]" multiple="multiple" size="7">
  <option value="0">Sunday</option>
  <option value="1">Monday</option>
  <option value="2">Tuesday</option>
  <option value="3">Wednesday</option>
  <option value="4">Thursday</option>
  <option value="5">Friday</option>
  <option value="6">Saturday</option>
</select>
Note the square brackets inside the "name" attribute. Now, if your user picks Thursday, Friday, and Saturday, the reply will look something like

Code: Select all

available[]=6&available[]=4&available[]=5
which becomes

Code: Select all

$_GET['available'][0]=6;
$_GET['available'][1]=4;
$_GET['available'][2]=5;
and the $value variable will, itself, become an array. This will generate an error. </deadpan>

When this problem beat me up, I wound up doing this:

Code: Select all

if ($_GET) {
    foreach ($_GET as $index1 => $value1) {
        if (is_array($value1)) {
            foreach ($value1 as $index2 => $value2) {
                $get[$index1][$index2] = $elements->sanitize($value2);
            }    
        }    
        else {
            $get[$index1] = $elements->sanitize($value1);
        }    
    }    
    unset ($_GET);
}
if ($_POST) {
    foreach ($_POST as $index1 => $value1) {
        if (is_array($value1)) {
            foreach ($value1 as $index2 => $value2) {
                if (is_string($value2)) {
                    $post[$index1][$index2] = $elements->sanitize($value2);
                }    
            }    
        }    
        elseif (is_string($value1)) {
            $post[$index1] = $elements->sanitize($value1);
        }
    } # foreach ($_POST as $index1 => $value1)
    unset ($_POST);
}
which runs everything through a sanitizer function, even if a given key turns out to contain an array. (The trick of using square brackets inside a name attribute isn't mine. But, I don't remember where I saw it.)

Cheers!
dafydd

--
The word "instantiate" bugs me. "Create an instance of" is too hard to type?

Re: Cleaning all $_POST or $_GET - error in code

Posted: Tue Feb 01, 2011 10:02 pm
by s.dot
I generally only apply a function to strip slashes if magic_quotes_gpc is enabled.
Database escaping and HTML escaping should not be applied to all data.

Here's the function I use:

Code: Select all

/**
 * This function will recursively strip magic quotes if that setting is enabled 
 * in the target system.
 *
 * @param mixed $arr
 * @return mixed
 */
function stripGPC(&$arr)
{
	if (is_array($arr))
	{
		foreach ($arr AS $k => $v)
		{
			$arr[$k] = stripGPC($v);
		}
	} else
	{
		$arr = stripslashes($arr);
	}
	
	return $arr;
}

/**
 * Now we need to see if we need to run the stripGPC function, and run it if 
 * we need to.
 */
if (get_magic_quotes_gpc())
{
	foreach (array('_GET', '_POST', '_COOKIE') AS $sg)
	{
		${$sg} = stripGPC(${$sg});
	}
}