5.3.0-git
2014-09-23

[#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 (at) , chuck (at) horde (dot) org
Requester chuck (at) horde (dot) org
Created 2008-10-07 (2177 days ago)
Due
Updated 2012-10-16 (707 days ago)
Assigned
Resolved
Milestone
Patch No

History
2012-10-16 11:45:41 Jan Schneider Milestone ⇒
 
2011-04-20 02:48:10 Chuck Hagenbuch Comment #11 Reply to this comment
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.
2011-04-19 16:24:02 Jan Schneider Comment #10 Reply to this comment
Anyone?
2011-04-04 22:04:12 Jan Schneider Comment #9 Reply to this comment
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?
2011-04-04 18:24:59 Michael Slusarz Comment #8 Reply to this comment
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.
2011-04-04 12:13:25 Jan Schneider Comment #7 Reply to this comment
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.
2011-04-01 19:54:06 Jan Schneider Comment #6 Reply to this comment
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.
2011-04-01 19:05:21 Michael Slusarz Comment #5 Reply to this comment
Horde_Util::createTempDir()
Gollem_Api::setSelectlist()
These have nothing to do with security AFAIK - they are simply 
intended to create non-colliding identifiers.
2011-03-31 23:36:44 Jan Schneider Comment #4
Summary ⇒ Audit for inappropriate use of mt_rand()
Reply to this comment
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()
2010-09-24 22:15:03 Jan Schneider Milestone ⇒ 4
Version ⇒ Git master
 
2008-12-03 23:40:57 Michael Slusarz Comment #3 Reply to this comment
I can confirm the imp/mimp calls are fine.
2008-11-10 05:35:03 Chuck Hagenbuch Comment #2
New Attachment: mt_rand.txt Download
Reply to this comment
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.
2008-10-07 20:06:34 Chuck Hagenbuch Comment #1
State ⇒ Assigned
Patch ⇒ No
Milestone ⇒
Assigned to Horde DevelopersHorde Developers
Assigned to Chuck Hagenbuch
Queue ⇒ Horde Base
Summary ⇒ Audit for innappropriate use of mt_rand
Type ⇒ Bug
Priority ⇒ 2. Medium
Reply to this comment
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/