[#7447] Audit for inappropriate use of mt_rand()
Summary Audit for inappropriate use of mt_rand()
Queue Horde Base
Queue Version Git master
Type Bug
State Assigned
Priority 2. Medium
Owners Horde Developers, chuck@horde.org
Requester chuck@horde.org
Created 2008-10-07 (3939 days ago)
Due
Updated 2012-10-16 (2469 days ago)
Assigned
Resolved
Milestone
Patch No

Comments
Chuck Hagenbuch <chuck@horde.org> 2008-10-07 20:06:34
Shouldn't use mt_rand on its own to generate a key for a password 
reset or a new password or anything else that could be exploited:

http://www.suspekt.org/2008/08/17/mt_srand-and-not-so-random-numbers/

Chuck Hagenbuch <chuck@horde.org> 2008-11-10 05:35:03
These are the calls in HEAD that are potentially of concern. Most 
likely not all of them are; these are the ones I wasn't able to filter 
out without looking at context.

Michael Slusarz <slusarz@horde.org> 2008-12-03 23:40:57
I can confirm the imp/mimp calls are fine.

Jan Schneider <jan@horde.org> 2011-03-31 23:36:44
The question is, what else to we use (additionally?) as a secret or 
source of randomness? /dev/urandom is not available on all systems. 
Our pre-generated secret_key doesn't change.
In Horde_Support we use:
- php_uname('n') or ip address (not random, only to avoid collisions)
- uniqid() (with the more-entropy parameter a good candidate)
- zend_thread_id()/getmypid() (short)
- microtime() (predictable)

Horde_Oauth and Horde_Token use microtime() resp. time() for Nonces.

This needs to applied to:
Horde_Auth::getSalt(), genRandomPassword() (salt and password generation)
Horde_ActiveSync_State_Base::generatePolicyKey()
Horde_Secret::setKey()
Shout::genDeviceAuth()

And probably to share and object ids and resources too, since they 
could be used to share hidden shares/objects through a secret url:
Horde_Core_Imsp_Utils::synchShares()
Kronolith_Resource::addResource()
Turba_Driver::_makeKey()

I'm unsure about:
Kolab_Storage

Only if being anal for:
Horde_Form_Type_image::getRandomId()
Horde_Util::createTempDir()
Gollem_Api::setSelectlist()

Michael Slusarz <slusarz@horde.org> 2011-04-01 19:05:21
> Horde_Util::createTempDir()
> Gollem_Api::setSelectlist()

These have nothing to do with security AFAIK - they are simply 
intended to create non-colliding identifiers.

Jan Schneider <jan@horde.org> 2011-04-01 19:54:06
>> Horde_Util::createTempDir()
>> Gollem_Api::setSelectlist()
>
> These have nothing to do with security AFAIK - they are simply 
> intended to create non-colliding identifiers.

It could theoretically be used for information leakage if another 
application or host is using the same temp directory. But like I said, 
this is really picky.

Jan Schneider <jan@horde.org> 2011-04-04 12:13:25
Any opinions how to generate more random keys best? I tend to simply 
use Horde_Support_Randomid everywhere. We can always improve that in a 
single place if necessary then.

Michael Slusarz <slusarz@horde.org> 2011-04-04 18:24:59
> Any opinions how to generate more random keys best? I tend to simply 
> use Horde_Support_Randomid everywhere. We can always improve that in 
> a single place if necessary then.

This seems like the correct thing to do.

Jan Schneider <jan@horde.org> 2011-04-04 22:04:12
In Horde_Auth::getSalt(), we always use an MD5 hash as the basis for 
the salt, unless a seed is specified. What I don't get is, why we 
sometimes hash mt_rand() directly, and sometimes 2 or 3 calls to 
mt_rand(), converted to hex strings?

Jan Schneider <jan@horde.org> 2011-04-19 16:24:02
Anyone?

Chuck Hagenbuch <chuck@horde.org> 2011-04-20 02:48:10
I think it has to do with how much random input the salt is expecting. 
I'd have to do some research to be sure, though.