Link to home
Start Free TrialLog in
Avatar of groovyjon
groovyjon

asked on

Sanitising all global variables in php

In some PHP code we've inherited, the PHP admin value register_globals is set to ON. Some parts of the code address variables as $_POST['var'] and some parts just use $var.

In order to stop sql injection attacks, I would like to sanitise all POST and GET parameters using mysql_real_escape_string.

I've written a function that goes at the top of each page to loop through the $_POST and $_GET arrays to sanitise those values. But how do I do it for the variables that have been inserted into the page as $var?

Trawling through the code changing all instances of $var to $_POST['var'] is going to be a nightmare as there is a lot of code. I don't want to sanitise the whole of the GLOBALS array either in case it has any unwanted side-effects.

Any suggestions?
Avatar of rfportilla
rfportilla
Flag of United States of America image

If you have a function that will accept the $_REQUEST and returns a clean version then that's what I would do.  I wouldn't try to "globaly" sanitize them or put the function at the top of each page.  If you forget to put it at the top of a page then you have a security hole.  

You should get into the habit of checking the data when you access it.  Every time you access the data, run your function.  
You would want something like the following except $key would need to refer to the actual variable name.




At the beginning of every page place:

foreach($_POST as $key=>$val)
{
    //do your exscaping or whatever here.

    //overwrite the variable.
    $key = $val;
}
foreach($_GET as $key=>$val)
{
    //do your exscaping or whatever here.

    //overwrite the variable.
    $key = $val;
}
Avatar of groovyjon
groovyjon

ASKER

rfportilla - I agree that the data should be checked at the point of usage, but remember this is a large amount of inherited code, so trawling through changing every bit of data isn't feasible, hence the global solution. I'm putting the sanitisation code in a separate file that's included in every page anyway so no chance of missing it.
professionalcomputersolutions - I'm not sure I follow. I already have a function that overwrites the values in the _POST and _GET arrays using a foreach loop just like you've shown. But how about the $var's accessed directly in the page? I'm not sure what your $key=$val statement will achieve.

To use an example, assume I have a _POST array as follows:

$_POST['var1']='abc';
$_POST['var2']='xyz';

I can sanitise $_POST['var1'] and $_POST['var2'] easily enough. But code that accesses $var1 and $var2 still see the original un-sanitised values. How do I sanitise those?
I don't know if that is possible.

I would recommend using find/replace in dreamweaver or similar.
replacing the $var with $_POST['var']
Turn off register globals

I never use register globals and disagree with it. I like to have full control of my code. accessing the variables through the get or post works both ways and in my opinion is the way to do things. Better off fixing now and preventing future issues.
Certainly changing the code would be a better solutions. But there are so many variables in so many pages. Some are GET variables, some are POST variables, and some are neither. So it's going to take more than just doing a find/replace
If you don't know the variable names, then there is no solution.  You have to know what you are modifying.  

Ultimately, I think you have your solution already.  You have the function in a file that you are including at the top of every page.  Just run the function in your include file and voila.  

In any case, user data is going to be accessed as $_REQUEST, $_GET, $_POST, or $_COOKIE.  I don't know of any other way right off, (except maybe an upload file, but that would be really weird).  

ASKER CERTIFIED SOLUTION
Avatar of groovyjon
groovyjon

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Lines 11 and 12 above are the relevant lines for what I was trying to achieve.
I know you've been told this, but the code really needs to be updated to NOT use global variables.

That said, you can use "variable variables" and just loop through any of the superglobals like $_POST, $_GET, etc (those two are normally the only culprits).  The code is something like this:
foreach($_POST as $key=>$val) {
    //clean the variables here.... 
    $$key = sanitise_var($val);
}

Open in new window


If you've got a $_POST var, "$_POST['myVar']", then the script will get (a sanitized) "$myVar".  The way variable-variables work, is the inner part, "$key" evaluates to "myVar".  The next pass turns it into the variable, "$myVar".
Thanks crazedsanity, I think that's basically what I have in my code above.

We will look to update the code properly at some point but it's not a trival job. This is just a quick fix for now.
I'm not certain that looping through a superglobal ($_GET, $_POST, etc) will give you the desired results, as it may not affect the derived variable.  I can't find anything that says what the behavior is, but I'm pretty sure you can sanitize $_GET['var'] and still have an un-sanitized $var.  Does that make sense?

Another thing to consider: the php.ini declares the order that $_GET and $_POST are handled; you should sanitize in that same order in the unlikely event that $_POST['var'] and $_GET['var'] both exist.
Good point about the ordering of _GET and _POST.

As for the derived variables, no sanitizing the _GET leaves the derived variable unchanged - that was my original problem. But that is addressed now in lines 11 and 12 of the code I posted.
Looks like I completely overlooked your usage of variable-variables as well.  I think your code should probably do the trick.

If the software in question is something that was not custom-built, there may well be an updated version out there that is free of the register_globals problem.  Sadly, many of these codebases aren't built for upgrade, so it might not work anyway... but it is worth looking into.
Own solution.