PHPsec.org Security Guide considered harmful




In January 2005 the PHP Security Consortium was founded. Their mission was described as promoting secure programming practices within the PHP community through education and exposition while maintaining high ethical standards.

Now, 6 months later, more and more people start to question, if the consortium has achieved anything, beside promoting their own companies and replacing the speakerlist on PHP conferences with people among their own. Although it is very interesting to see, how a little bit media hype, initiated by a friendly magazine and the presence of Zend within the consortium, has made it popular, it is more interesting to have a look into their PHP Security Guide, which has been their only project for about 5 months.

While the guide contains a few recommendations that are either unrealistic in shared environments or fill admins with terror, like putting database credentials into environment variables, which is obviously not a good idea with thousands of open phpinfo() scripts, there are also atleast two security bugs in their recommended practices.

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)

The first big bug is within the recommended solution for Cross Site Request Forgery vulnerabilities. In the following example, which was copied directly from the Security Guide, a random token is generated for a HTML form and put into a $_SESSION variable to protect against the typical CSRF situation.
<?php

session_start();

if (isset($_POST['message']))
{
    if ($_POST['token'] == $_SESSION['token'])
    {
        $message = htmlentities($_POST['message']);

        $fp = fopen('./messages.txt', 'a');
        fwrite($fp, "$message<br />");
        fclose($fp);
    }
}

$token = md5(uniqid(rand(), true));
$_SESSION['token'] = $token;

?>

<form method="POST">
<input type="hidden" name="token" value="<?php echo $token; ?>" />
<input type="text" name="message"><br />
<input type="submit">
</form>

<?php

readfile('./messages.txt');

?>

Unfortunately this solution does not fix the problem at all, because there is no check that $_SESSION['token'] has any value at all. This means, whoever is tricked into sending the request, will most probably never have visited the attacked site and therefore his session does not contain any 'token'. And of course comparing two empty variables with each other will always be true. And that means the same attack, is still possible.

The second security bug in the PHP Security Guide is hidden in the user space session handler, that is presented in chapter 5.1. In this chapter it is recommended to write your own session handler because otherwise your session data will be stored in /tmp for all VHOSTs and this is of course bad. Beside the fact, that the guide fails to mention the possibility, to have an own session save dir for every VHOST, it presents a solution in form of an SQL Userspace Session Handler. Nearly the same code can be found in the popular server configuration GUI Confixx. And until recently it contained the same security bug.
function read($id)
{
    global $_sess_db;

    $sql = "SELECT data
            FROM   sessions
            WHERE  id = '$id'";

    if ($result = mysql_query($sql, $_sess_db))
    {
        $record = mysql_fetch_assoc($result);

        return $record['data'];
    }

    return false;
}

This little snippet is the Session Read Handler from the example. For anyone it should be quite obvious that here the session identifier within the $id variable is inserted into a dynamicly created SQL query without any kind of validation, filtering or escaping. A classical example for an SQL injection vulnerability, within an example for recommended secure coding practices.

Closing with a quote from a member of the PHP Group: "Mistakes happen I guess, but a 'consortium' writing a security guide with errors this obvious, doesn't fill me with confidence."

UPDATE: I suggest that you also read Chris Shiflett's comments about this article in his blog. It is an entertaining read, because he says that I haven't understood the CSRF problem, and blames me for not initialising the variables in my example. Which is kinda strange, because the example was copied one to one from his security guide and therefore any error in it, is his fault. And well I forgive him his unreasonable attacks on my person in his blog.

Stefan Esser

References: PHPsec.org Security Guide
© Hardened PHP Project