php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #62835 solr extension reseting random engine
Submitted: 2012-08-16 08:12 UTC Modified: 2014-02-14 21:18 UTC
From: glen at delfi dot ee Assigned: omars (profile)
Status: Closed Package: solr (PECL)
PHP Version: Irrelevant OS:
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: glen at delfi dot ee
New email:
PHP Version: OS:

 

 [2012-08-16 08:12 UTC] glen at delfi dot ee
Description:
------------
(i wrote about this  directly to author (Israel Ekpo <iekpo@php.net>) on 2012-
07-06 because the bugtracker was down at that time (and it still is), but author 
has not responded either)

marking this as security, because security checks usually involve randomizer, 
and if randomizer is predictable, the security protections can be avoided

---

hi

i'm sorry to write you directly, but as pecl site is broken (again!? no 
wonder!), there's no way to report bugs :(

today i suffered weird race condition in my systems,
and debugging leaded it back to solr pecl extension,
i.e removing pecl extension from system, made it stable again.

so as symptoms lead to believe somebody fcks with randomizer,
i grepped the source the result is this patch that fixes the problem

http://cvs.pld-linux.org/cgi-bin/viewvc.cgi/cvs/packages/php-pecl-solr/do-not-
screw-with-random-seed.patch

please consider releasing new version asap,
as is it's seriously broken for extension to mess with randomizer setup



Patches

do-not-screw-with-random-seed.patch (last revision 2012-08-29 11:27 UTC by glen at delfi dot ee)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-08-16 08:14 UTC] glen at delfi dot ee
ps: patch contains more comments and description
 [2012-08-16 09:27 UTC] pajoye@php.net
-Type: Security +Type: Bug
 [2012-08-16 09:27 UTC] pajoye@php.net
> i'm sorry to write you directly, but as pecl site is broken (again!? no 
> wonder!), there's no way to report bugs :(

Pardon? bugs.php.net is the place where pecl's issues are reported. Just like what 
you just did.

I moved this to a non secure related issue as none of rand or mt_rand are security 
related anyway.
 [2012-08-16 09:29 UTC] pajoye@php.net
btw, could you attach the patch to this issue please?
 [2012-08-16 09:32 UTC] glen at delfi dot ee
you're missing the fact that it WAS BROKEN, Ferenc fixed it today:


On Thu, Aug 16, 2012 at 10:04 AM, Elan Ruusamäe <glen@delfi.ee> wrote:

    http://pecl.php.net/bugs/search.php?
cmd=display&package_name[]=solr&status=Open

    says to contact here

    link came from this page http://pecl.php.net/package/solr
     


Hi Elan,

We merged the pecl bugtracker into the main bugtracker on bugs.php.net.
That package had his bugtracker url still set to the old url, I've just fixed 
it, thanks for reporting it.

I've checked the database, and we have two other package(ncurses and funcall) 
having the same issue(having bug_link explicitly set to 
http://pec.php.net/bugs/), I also fixed those.

-- 
Ferenc Kovács
@Tyr43l - http://tyrael.hu
 [2012-08-16 09:34 UTC] glen at delfi dot ee
you should really get to understand how your infra works before blaming or 
requesting impossible stuff

it is not possible to attach any file to private bugs

and i don't understand why you can't take the patch from url:
http://cvs.pld-linux.org/cgi-bin/viewvc.cgi/cvs/packages/php-pecl-solr/do-not-
screw-with-random-seed.patch
 [2012-08-16 09:37 UTC] glen at delfi dot ee
> I moved this to a non secure related issue as none of rand or mt_rand are 
security related anyway.

regarding security: i explained if someone's security defense is based on 
randomizer (generates login cookie based on rand(), for example), and if 
randomizer is predictable, then the security defense is broken as well. so this 
is security *related*
 [2012-08-18 15:17 UTC] felipe@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: iekpo
 [2012-08-29 08:50 UTC] pajoye@php.net
@glen at delfi dot ee

With all respects, if an app is still using rand (or mt_rand) for any security 
related area, then the app has big troubles, no matter what solr does or not.

As it is clearly a bug (somehow security), it is not critical to be marked as 
private or to be discussed only in security@


Btw, look here too:

http://lxr.php.net/search?q=&defs=&refs=srand&path=&hist=&project=PECL

I did not check all of them :)
 [2012-08-29 11:18 UTC] glen at delfi dot ee
couldn't agree more that apps relaying only on rand() have poor security :)

indeed, those lxr refs initializing with time(), time(NULL), time(thisTime), and 
even time(seed) from mysqlng (seed is microseconds from gettimeofday there) are 
evil as well and should be fixed, or at least commented why exactly that code 
changes something so global as randomizer state.

that srand(0) there is at least on purpose:
470#if defined(DEBUG)
471  /* To make it random number generator repeatable to ease testing. */
472  srand(0);
473#endif

php_bloomy.c doing srand based on GENERATE_SEED() does not seem well either, as 
php engine provides initial randomizer setup, perhaps that GENERATE_SEED macro 
should be made private so extensions can't (ab)use it...
 [2012-08-29 11:31 UTC] glen at delfi dot ee
uploaded patch to bugtracker. duplicating patch header as probably nobody 
bothers to read it (from patch itself) :)

---
Do not initialize the seed with srand or mt_srand, it is already initialized
since PHP 4.2.0 (See the docs). And if you initialize it with time or microtime
you make it only more worse (predictable).

And initializing random seed with predictable value is extremely stupid.

I had situation that three machines simulatenously produced temporary directory
names based on random generator, and they all got same results, this is utterly
screwed up up the application!

Jul  6 10:37:26 segusilm php.fcgi: PHP Fatal error:  Uncaught exception 
'Exception' with message 'Can't mkdir '/tmp/foo_UZNHCMUDWZ/archive': mkdir(): 
File exists' in /usr/share/foo/lib/plugins/Plugin.php:127
Jul  6 10:37:26 lordi php.fcgi: PHP Warning:  
unlink(/tmp/foo_UZNHCMUDWZ/archive/title.inc): No such file or directory in 
/usr/share/foo/lib/helper/OutputHelper.php on line 68

it is strongly recommended for PHP application developers to keep their fingers
away from srand() or mt_srand() and to never ever use rand() or mt_rand() for
cryptographic secrets:
http://www.suspekt.org/2008/08/17/mt_srand-and-not-so-random-numbers/
 [2013-06-03 08:21 UTC] glen at delfi dot ee
PING?!
 [2013-10-26 18:27 UTC] glen at delfi dot ee
any progress with this?
 [2014-02-14 21:18 UTC] omars@php.net
-Assigned To: iekpo +Assigned To: omars
 [2014-02-15 17:31 UTC] omars@php.net
Automatic comment on behalf of omars
Revision: http://git.php.net/?p=pecl/search_engine/solr.git;a=commit;h=faf1fa5c3b4ccecda456c0ebe7f9955797028f35
Log: Fixed Bug #62835: removed srand() call from module request init
 [2014-02-15 17:31 UTC] omars@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 20 02:01:29 2024 UTC