php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #51436 LCG entropy fix insufficient, uniqid leaks entropy, leads to weak session IDs
Submitted: 2010-03-30 12:38 UTC Modified: 2015-04-05 05:16 UTC
Votes:5
Avg. Score:3.0 ± 1.3
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:0 (0.0%)
From: andreas at andreas dot org Assigned: pajoye (profile)
Status: Closed Package: *Encryption and hash functions
PHP Version: 5.3.2 OS: all
Private report: No CVE-ID: None
 [2010-03-30 12:38 UTC] andreas at andreas dot org
Description:
------------
PHP utilizes a cryptographically weak random number generator to produce session ID information.  Additionally, not enough entropy is used for the initial seeding of the RNG, and some of the entropy can leak by careless use of the uniqid() PHP function.  Under certain circumstances, these individual weaknesses interact and reduce the number of possible values of a PHP session ID so much that exhaustive search for a valid session ID against the web server becomes feasible.

I suggest to make sure that a cryptographically secure RNG is used for session ID generation, sufficient entropy is used to seed the RNG, and to change the uniqid() function to always return a hashed value.

A complete discussion of why I think the code is vulnerable, including estimates on the attack effort, is available from http://berlin.ccc.de/~andreas/php-entropy-advisory.txt


Patches

session_entropy_docs_php_ini_default_off_still (last revision 2010-03-31 02:43 UTC by philip@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-03-30 20:19 UTC] pajoye@php.net
On a related note, we should document session.entropy-file in a better way. Maybe this page should be a good place to inform the users about this setting and why it should always be used:

http://www.php.net/manual/en/session.installation.php

Thanks Rasmus for the notice.
 [2010-03-31 01:08 UTC] philip@php.net
Regarding session.entropy_file and session.entropy_length, please clarify this 
topic a bit and ideally include an example for Windows users. I see words like 
ksecdd.sys and CryptoAPI but am unsure how these might apply to 
session.entropy_file.

And, what are the downsides of using these options... performance?
 [2010-03-31 04:43 UTC] philip@php.net
The following patch has been added/updated:

Patch Name: session_entropy_docs_php_ini_default_off_still
Revision:   1270003407
URL:        http://bugs.php.net/patch-display.php?bug=51436&patch=session_entropy_docs_php_ini_default_off_still&revision=1270003407
 [2010-03-31 05:04 UTC] philip@php.net
As for these session.entropy directives, due to compatibility issues I'm unsure 
how best to include these recommended values in php.ini-* so the following 
patch[1] adds information where appropriate, although it does not change the 
default values. One trouble: it breaks convention, as other directives are in 
fact changed (and not only recommended). This patch only solves this bug through 
documentation, which may or may not be our ultimate solution.

We still need to discuss whether changing the default php.ini-* values is 
appropriate, and the potential impact (e.g. Windows) it would have. And of 
course explore alternative options that essentially don't "require" 
/dev/urandom. Like, Rasmus/Scott mentioned something about using OpenSSL's 
existing abstraction layer to do it.

And lastly, while documenting we should describe:
- Briefly mention the difference between /dev/urandom and /dev/random
- Talk about performance issues
- Alternatives to /dev/random (e.g. EGD, hardware, ...)
- Mention which Operating Systems lack /dev/random (Windows, and Solaris 8 and 
below come to mind)

[1] Patch name: session_entropy_docs_php_ini_default_off_still
 [2010-03-31 20:03 UTC] rasmus@php.net
Automatic comment from SVN on behalf of rasmus
Revision: http://svn.php.net/viewvc/?view=revision&revision=297232
Log: Set session.entropy_file to /dev/urandom or /dev/arandom by
default if present at compile-time.  Addresses part of bug #51436
 [2010-03-31 20:30 UTC] rasmus@php.net
I have switched the default in trunk to either /dev/urandom or /dev/arandom if it 
exists.  We actually already had a check for it in Zend for the zend_mm_random() 
function,  Whether we backport this to 5.3 or just improve the documentation for 
that setting is up to Johannes, I think.
 [2010-04-07 17:21 UTC] andreas at andreas dot org
I strongly suggest backporting.  Also, the fact that uniqid() values are predictable too needs addressing.
 [2010-04-07 17:43 UTC] pajoye@php.net
Well, the easiest to "backport" something now and here is to use the given settings. You can do it right now.
 [2010-04-07 17:44 UTC] pajoye@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: pajoye
 [2010-04-07 17:44 UTC] pajoye@php.net
And assigned to me, almost done with the patch we discussed.
 [2010-04-09 17:51 UTC] crrodriguez at opensuse dot org
I think uniqid() should also use zend_mm_random()-like random value when 
more_entropy is set to true instead of the LCG ...
 [2010-04-09 18:18 UTC] pajoye@php.net
That's the idea but not using zend's mm which is incomplete.
 [2010-04-09 18:41 UTC] crrodriguez at opensuse dot org
I think trying RAND_pseudo_bytes() if -lcrypto is found in the system first and 
then your_own_function ight be a suitable approach.
 [2010-04-09 19:05 UTC] pajoye@php.net
RAND_pseudo_bytes does pretty much the same anyway, but I would prefer to give a possible not to use openssl first.

Also this exact function may not be crypto safe. It is not a problem for the session but that will then not solve the need of a crypto safe function.
 [2010-06-08 15:56 UTC] pajoye@php.net
I added support for the entropy source to 5.3 and trunk.

See http://svn.php.net/viewvc?view=revision&revision=300273 and
http://svn.php.net/viewvc?view=revision&revision=300278

Will close the bug once we also have defined the default hash (would like to make it a default in trunk as well).
 [2014-07-15 11:50 UTC] yohgaki@php.net
I suppose this can be closed, can't this?
 [2015-04-05 05:16 UTC] pajoye@php.net
-Status: Assigned +Status: Closed
 [2015-04-05 05:16 UTC] pajoye@php.net
Thank you for your bug report. This issue has already been fixed
in the latest released version of PHP, which you can download at 
http://www.php.net/downloads.php


 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Sun Mar 07 02:01:25 2021 UTC