Difference between revisions of "Register globals"

From GeeklogWiki
Jump to: navigation, search
(About register_globals and how to deal with it)
 
(Added category; cosmetics)
 
(3 intermediate revisions by 2 users not shown)
Line 2: Line 2:
  
 
Despite the common conception that register_globals is "evil", it is not a security issue ''per se'', as long as you follow a few simple rules (see below). In all the years that Geeklog required register_globals to be "on", there was only '''one''' security issue that was a result of that setting.
 
Despite the common conception that register_globals is "evil", it is not a security issue ''per se'', as long as you follow a few simple rules (see below). In all the years that Geeklog required register_globals to be "on", there was only '''one''' security issue that was a result of that setting.
 +
 +
On a side note, the register_globals setting will be removed from PHP 6, i.e. it will always be "off" and there will be no way to switch it back "on". So now would be a good time to review your code for dependency on the register_globals setting ...
  
  
Line 36: Line 38:
  
 
This is actually a better programming style anyway, not only from a security point of view. You should adapt this style in your own interest.
 
This is actually a better programming style anyway, not only from a security point of view. You should adapt this style in your own interest.
 
  
 
=== Switch on error reporting ===
 
=== Switch on error reporting ===
Line 59: Line 60:
  
 
The notices caused by the language files will prevent you from logging in to your site, so you may want to comment out the error_reporting line only during the debugging phase of your development cycle.
 
The notices caused by the language files will prevent you from logging in to your site, so you may want to comment out the error_reporting line only during the debugging phase of your development cycle.
 +
 +
 +
== Use typesafe comparison operations ==
 +
The content of the superglobal arrays ($_GET, $_POST, $_REQUEST etc) are all strings, as are any variables created by register_globals. As this is the case, you can secure your code further by using typesafe comparisons:
 +
 +
<pre>
 +
if ($authorized === TRUE)
 +
{
 +
  // Only matches if $authorized contains an actual boolean true value.
 +
}
 +
</pre>
 +
 +
== Hidden problems with register_globals ==
 +
Register globals allows you to inject values into arrays in a non-detectable fashion:
 +
 +
<pre>
 +
$sids[] = "123";
 +
foreach ($sids as $sid)
 +
{
 +
  // Delete article by sid
 +
}
 +
</pre>
 +
 +
With the above code, a url such as:
 +
<pre>somepage.php?sids[]=1&sids[]=2</pre>
 +
will inject additional values into the arrays.
 +
  
  
Line 91: Line 119:
  
 
The replacement for those, as mentioned above, are the $_GET and $_POST arrays and only those should be used. They were introduced in PHP 4.1.0, which has also been the minimum requirement for Geeklog for a while now.
 
The replacement for those, as mentioned above, are the $_GET and $_POST arrays and only those should be used. They were introduced in PHP 4.1.0, which has also been the minimum requirement for Geeklog for a while now.
 +
 +
 +
[[Category:Development]]

Latest revision as of 13:11, 7 March 2008

Starting with version 1.4.0, Geeklog no longer requires to have PHP's register_globals option set to "on".

Despite the common conception that register_globals is "evil", it is not a security issue per se, as long as you follow a few simple rules (see below). In all the years that Geeklog required register_globals to be "on", there was only one security issue that was a result of that setting.

On a side note, the register_globals setting will be removed from PHP 6, i.e. it will always be "off" and there will be no way to switch it back "on". So now would be a good time to review your code for dependency on the register_globals setting ...


Initialising variables

The main rule to avoid problems with the register_globals setting is pretty simple:

Initialise your variables!

The PHP 4.1.0 release notes had the following example to illustrate what's bad about register_globals=on:

<?php
if (authenticate_user()) {
  $authenticated = true;
}
...
?>

With register_globals=on, it's simple to override the authentication check by adding "authenticated=true" to the URL of this script, e.g.

http://www.example.com/authenticate.php?authenticated=true

However, that problem is easily fixed:

<?php
$authenticated = false;
if (authenticate_user()) {
  $authenticated = true;
}
...
?>

There. You can pass whatever you want in the URL now - initialising the $authenticated variable fixed the problem.

This is actually a better programming style anyway, not only from a security point of view. You should adapt this style in your own interest.

Switch on error reporting

On your development system (never do that on a production site), it's recommended that you switch on error reporting to be made aware of issues with uninitialised variables.

In your php.ini file, set

error_reporting   = E_ALL
display_errors         = On
display_startup_errors = On

... and don't forget to restart your webserver after you've made those changes.

Then, in Geeklog's lib-common.php, find the line

error_reporting( E_ERROR | E_WARNING | E_PARSE | E_COMPILE_ERROR );

and comment it out or remove it.

Note: Removing the error_reporting line from lib-common.php will throw up quite a few notices about uninitialised variables in the language files. Those are historic issues that can't be fixed easily but are not a security hazard. Just make sure not to repeat these design issues in your own code ...

The notices caused by the language files will prevent you from logging in to your site, so you may want to comment out the error_reporting line only during the debugging phase of your development cycle.


Use typesafe comparison operations

The content of the superglobal arrays ($_GET, $_POST, $_REQUEST etc) are all strings, as are any variables created by register_globals. As this is the case, you can secure your code further by using typesafe comparisons:

if ($authorized === TRUE)
{
  // Only matches if $authorized contains an actual boolean true value.
}

Hidden problems with register_globals

Register globals allows you to inject values into arrays in a non-detectable fashion:

$sids[] = "123";
foreach ($sids as $sid)
{
   // Delete article by sid
}

With the above code, a url such as:

somepage.php?sids[]=1&sids[]=2

will inject additional values into the arrays.


Use $_GET, $_POST, and $_REQUEST

The above explained how to avoid security issues when register_globals is set to "on". Of course, initialising your variables won't make your code work with register_globals=off.

Values passed from user interaction, i.e. when the user clicks on a URL or submits a form, are always passed in the global $_GET and $_POST arrays. The variables you're looking for exist as named array elements in those arrays, e.g.

  • $_GET['page'] from a URL like /index.php?page=2
  • $_POST['title'] from a form that contains a comment with a "title" field

Note that a form usually has a method="POST" attribute which means that the form is sent as a HTTP POST request. Consequentially, the $_GET array will be empty in such a case and all the form's values will only exist in the $_POST array.

The $_REQUEST array contains both the contents of the $_GET and the $_POST array. This comes in handy sometimes when a certain portion of your code will have to handle something that could come in via either a GET or a POST request.

However, the $_REQUEST should be used sparingly and with caution, as it may make it easier for an attacker to submit manipulated requests. For example, when your form handling code only looks at the $_REQUEST array, it's also possible to submit all the form's parameter through one GET request which, in some situations, is easier to send than a POST request.

The security implications of the $_REQUEST array are only mild, though. It's recommended, however, that you write your code such that you always know whether the incoming data is in the $_GET or $_POST array, which automatically helps to improve the clarity of your code.


Switching off register_globals

On your development system, you should switch off register_globals in your php.ini file:

register_globals = Off


register_long_arrays

A somewhat related setting has been introduced with PHP 5: register_long_arrays, when set to "on", re-introduces the long names of the $_GET and $_POST arrays, i.e. $HTTP_GET_VARS and $HTTP_POST_VARS. These long-named arrays are disabled by default as of PHP 5 and any code relying on them will therefore fail.

The replacement for those, as mentioned above, are the $_GET and $_POST arrays and only those should be used. They were introduced in PHP 4.1.0, which has also been the minimum requirement for Geeklog for a while now.