Variable initialization in PHP
This article was published as the Month of PHP Security Submission.
Introduction
Consider the following code:
<?php if (authUser($_POST["login"], $_POST["password"])) { $auth = true; } if ($auth) { echo "Secret\n"; } ?>
You can easily spot the vulnerability in it. The $auth
variable is not initialized in all cases so it can be spoofed from outside. PHP defines a way to handle uninitialized variables (unlike C language for example), they all have the null
value. The problem is that the variable can be initialized from other sources:
- Previous code
- Included file
- From outside if
register_globals
is enabled - By using of
extract
on an untrusted variable
Defense
The defense against this vulnerability is simple – always initialize all variables:
<?php $auth = false; if (authUser($_POST["login"], $_POST["password"])) { $auth = true; } ?>
Now if $auth
contained anything (or nothing) then it is always reset to false
. It is a good idea to initialize variables unconditionally. The following code would work but it is not so robust:
<?php if (authUser($_POST["login"], $_POST["password"])) { $auth = true; } else { $auth = false; } ?>
Now consider that someone would like to check also the IP address and do it in this way:
<?php if ($_SERVER["REMOTE_ADDR"] == "127.0.0.1") { if (authUser($_POST["login"], $_POST["password"])) { $auth = true; } } else { $auth = false; } ?>
The $auth
variable can be spoofed again.
Note: The attacker must know the variable name to spoof – he can guess it, get it from some error message, brute-force it, but most commonly he gets it from the source code (open-source applications or a former employee).
Disabling register_globals
It is important to mention that disabling register_globals
is not a defense against this attack. It only closes the most common attack vector. It is of course a good idea to disable it but good applications that initialize all variables work independently of register_globals
value. Thus, it is not necessary to emulate disabling of register_globals
by some variation of the following code:
<?php // never use this kind of code if (ini_get("register_globals")) { foreach ($_REQUEST as $key => $val) { unset($$key); } } ?>
Spotting of uninitialized variables
PHP has a mechanism to spot uninitialized variables – it issues E_NOTICE
level error with most uses of uninitialized variables. There are however some problems with it:
- It does not warn about assigning to an uninitialized array.
- It warns about accessing non-existing index in properly initialized array.
- It is issued in runtime.
E_NOTICE does not warn about assigning to an uninitialized array
The first problem is most serious, consider the following code:
<?php $config["password"] = "pwd"; if (isset($_POST["password"]) && $_POST["password"] == $config["password"]) { echo "Secret information.\n"; } ?>
It issues no notices if an attacker spoofs the $config
variable. If he passes a string then the code is interpreted like this:
<?php $config[0] = "p"; // $config is a string so [] is used to access bytes in the string // "password" is converted to number 0 because string positions are always integers // only one byte can be written to [0] so "pwd" is interpreted as "p" if (isset($_POST["password"]) && $_POST["password"] == $config[0]) { echo "Secret information.\n"; } ?>
Now it is enough to guess the first character of password and send it along with spoofed $config
.
E_NOTICE warns about accessing non-existing index in properly initialized array
The second problem is not security related but code-brevity related. Following code would issue a notice even if it works well and cannot be fooled:
<input name="search" value="<?php echo htmlspecialchars((string) $_GET["search"]); ?>" />
With notices enabled, we must rewrite it to a longer code with the same functionality:
<input name="search" value="<?php if (isset($_GET["search"])) { echo htmlspecialchars((string) $_GET["search"]); } ?>" />
Another example of this thoroughness – following code is perfectly valid and does what we expect from it (count group members):
<?php $groups = array(); foreach (getData() as $data) { $groups[$data["id_group"]]++; } ?>
With notices enabled, it must be rewritten like this:
<?php $groups = array(); foreach (getData() as $data) { if (!isset($groups[$data["id_group"]])) { $groups[$data["id_group"]] = 0; } else { $groups[$data["id_group"]]++; } } ?>
I would not say that this code is more clear and less error-prone (there is already one error included).
E_NOTICE is issued in runtime
The third problem of notices is security-related. If some usage of uninitialized variable is not spotted during the development then an attacker can use it. It is nice that you are informed about the usage of uninitialized variable from the error log but if it is used for an attack then it is too late. It is possible to make notices fatal by set_error_handler
but it is not worth it for most applications.
Better spotting of uninitialized variables
I would not recommend disabling notices. However, it does not solve all problems and requires writing of more thorough code as you have seen. Luckily, there is a better way to spot uninitialized variables that solves all three problems of notices. It has a name php-initialized. It is a tool for analyzing PHP source code to spot uninitialized variables. It does not run the code and has some limitations but it can be used to check the code routinely for example after refactoring or before commit.
The biggest limitation is that only the current block is considered as the variable scope. Thus, the following code would complain about uninitialized $auth
:
<?php if (authUser($_POST["login"], $_POST["login"])) { $auth = true; } else { $auth = false; } if ($auth) { echo "Secret\n"; } // prints: Uninitialized variable $auth on line 7 ?>
It is partially a political decision explained in this article – it is less error-prone to initialize variables unconditionally. Apart this limitation, the supported features covers nearly all parts of PHP.
Note: Author of this article is the main developer of php-initialized.
Do not use $_REQUEST
Variable spoofing does not involve only global variables. The $_REQUEST
variable can be filled by other source than a programmer assumes. It is thoroughly explained by Steffan Esser. I would only append that as the contents of $_REQUEST
can be affected by request_order
since PHP 5.3.0 then an application could not rely on the contents of $_REQUEST
and would not run on some configurations.
Summary
Good PHP application should always initialize all variables before usage. It is a good idea to turn off register_globals
but a good application should not rely on it. PHP offers an E_NOTICE
error level that can spot some uninitialized variables but it is not 100% reliable. Tool php-initialized solves the deficiencies of it.
Comments
Tomas:
I didn't use PHP for quite a long time, but assigning true/false to a variable as the only thing done in the if statement is a bit surprising. Are there any possible issues if you write just:
<?php
$auth = authUser($_POST["login"], $_POST["login"]);
// rest of the code
?>
This seems much more natural to me.. Just wondering :-).
Jakub Vrána
:
You are absolutely right, $auth = authUser() is much better in this simple case. The example shows only a bad design pattern. If there would be more conditions, database queries etc. then a simple assignment would be impossible.


Ondrej Galbavý:
or this way: $auth = (bool)authUser(); if it returns object with user and only boolean is requiredaaa:
>> Now if $auth contained anything (or nothing) then it is always reset to false. It is a good idea to initialize variables unconditionally. The following code would work but it is not so robustWhat I don't like about unconditional variant is that is seems to be like default (or more expected) value of the variable which can be misleading. Because of this I often prefer:
if($boolean)
$auth = true;
else
$auth = false;
Jakub Vrána
:
$auth = false really is the default value.


Koubas:
Why it's not worth it using set_error_handler? Non-fatal errors are one thing that bothers me about PHP. The language it self cannot decide, whether some error is critical or not, because even a warning means that something went wrong and application could be in an unpredictible state (assuming that no today's php programmer releases code throwing warnings that he knows about). Or is something wrong about set_error_handler mechanism itself?Miško:
Ďakujem, pekný článok.Diskuse je zrušena z důvodu spamu.

