CVE-2016-1902: Symfony SecureRandom

Overview

Recently the Symfony project published a security advisory to the SecureRandom class in their Security component that affects Symfony versions 2.3.0-2.3.36, 2.6.0-2.6.12, 2.7.0-2.7.8. On most sane systems there is no problem, but in the event that something goes wrong the SecureRandom::nextBytes() falls back to a custom random number generator which creates insecure random numbers.

Details

The nextBytes() function has three methods of RNG:

1. If the PHP 7 random_bytes() function exists, that is used.
2. If OpenSSL is installed on the system then openssl_random_pseudo_bytes() is used and the result is checked to ensure RNG succeeded.
3. If all of the above fail, a custom scheme is used. This is where the problem lies.

The custom scheme can take in a seed file which doesn't necessarily need to exist. If the file does exist then the inital RNG seed is read from that file, otherwise the seed is initialized with uniqid(mt_rand(), true).

Here is where the random number is generated:

$bytes = '';
while (strlen($bytes) < $nbBytes) {
    static $incr = 1;
    $bytes .= hash('sha512', $incr++.$this->seed.uniqid(mt_rand(), true).$nbBytes, true);
    $this->seed = base64_encode(hash('sha512', $this->seed.$bytes.$nbBytes, true));
    $this->updateSeed();
}


return substr($bytes, 0, $nbBytes);

Upon inspection we can see the bytes that are eventually returned to the user are generated from a SHA-512 hash. This is probably done to make the output look uniform. A red flag is raised that maybe whatever is being fed to hash() isn't uniformly random. What's actually being fed to the core of this algorithm is the concatenation of some counter, seed, and a uniqid(). Let's take a look at each of these individually:

• The counter starts out at 1 and is declared static. Since PHP is stateless, the counter is predictable if you know how many times the function has been called for your request.

• mt_rand() is used as the $prefix to uniqid(). The PHP documentation for both of these functions explicitly state that they should not be used for cryptographic purposes. uniqid() inparticular isn't meant to even be random -- just unique. At its core it's just a concatenation of prefix, seconds, microseconds, and php_combined_lgc() (the latter explained here).

• The seed can be something we don't know if the file already exists. There are two cases where we can predict the seed though:

  1. If the user provides a path that doesn't exist
  2. If the user provides a path to a file that the application does not have permission to read or write.

(Sidenote: Hugo's markdown generator doesn't support ordered vs unordered lists... what?)

Point #2 is kind of interesting here because both failure to read or write the file fail silently and because PHP isn't exactly a type-safe language, nothing explodes.

readSeed():

private function readSeed()
{
    return json_decode(file_get_contents($this->seedFile));
}

If file_get_contents() fails it returns FALSE and is passed to json_decode(). If json_decode() fails, it simply returns NULL and you have to manually check json_last_error() to see why it failed. So if the file can't be read, your seed is null.

updateSeed() fails very similarly. The result of file_put_contents() is never checked and will fail silently.

tl;dr

SecureRandom::nextBytes() will generate insecure random numbers if you're not using PHP 7, don't have random_compat, and OpenSSL fails for some reason.

Even if this isn't a very practical or exploitable bug, it shows how a weakness at the core of a framework can go indiscovered for so long (this one almost 3 years!). I also mainly did this writeup to note that this is my first of hopefully many more CVEs.

Timeline:

• Dec 14, 2015: Vulnerability identified and reported
• Dec 30, 2015: No response, requested follow-up
• Jan 14, 2016: Patch released for 2.3, 2.6, 2.7
• Jan 18, 2016: Security advisory published