• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 296
  • Last Modified:

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?
0
groovyjon
Asked:
groovyjon
  • 8
  • 3
  • 2
  • +1
1 Solution
 
rfportillaCommented:
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.  
0
 
Tyler LaczkoCommented:
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;
}
0
 
groovyjonAuthor Commented:
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.
0
NFR key for Veeam Backup for Microsoft Office 365

Veeam is happy to provide a free NFR license (for 1 year, up to 10 users). This license allows for the non‑production use of Veeam Backup for Microsoft Office 365 in your home lab without any feature limitations.

 
groovyjonAuthor Commented:
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?
0
 
Tyler LaczkoCommented:
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.
0
 
groovyjonAuthor Commented:
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
0
 
rfportillaCommented:
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).  

0
 
groovyjonAuthor Commented:
I know the variable names at the time of calling the function because they are the index values of $_POST[] etc. I ended up using the following code. It's a bit of a hack, but it works.


function sanitiseArray(&$arr) {
		foreach ($arr as $key => $value) {
			if(get_magic_quotes_gpc()) {  // prevents duplicate backslashes
				$arr[$key]=stripslashes($arr[$key]);
			}
			if (phpversion() >= '4.3.0') {
				$arr[$key]=mysql_real_escape_string($arr[$key]);
			} else {
				$arr[$key]=mysql_escape_string($arr[$key]);
			}
			global ${$key};
			${$key}=sanitise($value);
		}
		reset($arr);
	}

Open in new window

0
 
groovyjonAuthor Commented:
Lines 11 and 12 above are the relevant lines for what I was trying to achieve.
0
 
crazedsanityCommented:
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".
0
 
groovyjonAuthor Commented:
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.
0
 
crazedsanityCommented:
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.
0
 
groovyjonAuthor Commented:
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.
0
 
crazedsanityCommented:
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.
0
 
groovyjonAuthor Commented:
Own solution.
0

Featured Post

Granular recovery for Microsoft Exchange

With Veeam Explorer for Microsoft Exchange you can choose the Exchange Servers and restore points you’re interested in, and Veeam Explorer will present the contents of those mailbox stores for browsing, searching and exporting.

  • 8
  • 3
  • 2
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now