Stefan Esser Discusses Security Guide
11 Jul 2005This is an interesting (and unexpected) continuation of the ethics and security discussion. Ben just pointed me to an interesting article by Stefan Esser, who claims to have found two errors in the PHP Security Guide. In the article, Stefan states:
Before continuing, it should be mentioned, that these bugs where disclosed to atleast 5 (if not 6) members of the PHP Security Consortium during the last 2-3 months (and nothing has changed, yet).
I find this quite surprising, since the last correspondence I had with Stefan was when he declined to assist with the efforts of the PHP Security Consortium (several months ago when the group was being formed), citing a "conflict of interest." As the founder of the group, it seems like he would have tried to bring these issues to my attention before such public disclosure.
One of the errors he mentions is valid, although luckily a recent update to the guide addresses it. While writing the shared hosting chapter of my book, I wrote functions for storing sessions in a database that include a few best practices not covered in the guide, such as the use of mysql_real_escape_string() instead of mysql_escape_string(). The guide contains these new functions.
The other concern is due to an error in Stefan's interpretation of the technique, which suggests that we need to explain the technique more clearly in the guide. We do not promote the use of uninitialized variables, and this is exactly what is required in order for a vulnerability to exist in the example cited. Variables should always be initialized, and a user's session is no exception. For example, you might initialize a few different session variables when you begin a session:
session_start();
if (!isset($_SESSION['logged_in']))
{
$_SESSION['logged_in'] = FALSE;
}
if (!isset($_SESSION['token']))
{
$_SESSION['token'] = md5(uniqid(rand(), true));
}
The guide includes small sections of code intended to teach a specific technique, and these are not always complete examples. Despite the lack of a vulnerability in this particular case, the guide fails to adhere to the principle of Defense in Depth, so an update has just been made to the guide that checks for a valid token as follows:
if (isset($_SESSION['token']) &&
$_POST['token'] == $_SESSION['token'])
Although this safeguard is redundant, it can save the day if you forget to initialize your variables. Remember to develop with error_reporting set to E_ALL to help you identify the use of uninitialized variables.