Ticket #6293 (closed defect: fixed)

Opened 4 months ago

Last modified 4 months ago

phpass should use uniqid(), not getmypid()

Reported by: tellyworth Assigned to: anonymous
Priority: normal Milestone: 2.5
Component: General Version: 2.5
Severity: blocker Keywords: has-patch
Cc: westi

Description

class-phpass.php uses this code to generate a random string: 

$this->random_state = microtime() . getmypid();

It shouldn't, because (a) it reinvents the uniqid() wheel, and (b) getmypid() is evidently disabled on some locked-down PHP installs:

http://wordpress.org/support/topic/162121?replies=2

The patch changes it to call uniqid() instead.

Attachments

phpass-uniqid-r7392.patch (444 bytes) - added by tellyworth on 03/19/08 00:29:02.

Change History

03/19/08 00:29:02 changed by tellyworth

  • attachment phpass-uniqid-r7392.patch added.

03/19/08 04:18:42 changed by Viper007Bond

  • version set to 2.5.
  • severity changed from normal to blocker.

phpass is a 3rd party class and I believe the developers prefer not to modify them in most cases.

This may be a special case though as it's a blocker.

03/19/08 04:20:42 changed by Viper007Bond

Oh, also:

Warning

Process IDs are not unique, thus they are a weak entropy source. We recommend against relying on pids in security-dependent contexts.

03/19/08 04:23:16 changed by DD32

phpass is a 3rd party class and I believe the developers prefer not to modify them in most cases.

Cases where its a Security issue or It doesnt work properly can be classified as bugs, AFAIK, Its ok to fix bugs in the version WordPress uses, and submit the patches up stream for consideration, or a better fix.

03/19/08 04:46:07 changed by Viper007Bond

Yeah, I know. :)

03/19/08 16:59:21 changed by ryan

Solar Designer, author of phpass, is investigating.

03/19/08 19:32:28 changed by ryan

http://drupal.org/node/29706?page=1#comment-746593

03/19/08 19:41:29 changed by ryan

Solar Designer sent me a detailed reply, which I'll try to summarize here. In general, getmypid() is sufficient for this fallback code path even if disabled, but if we want to address the warning we can use this: 

$this->random_state = microtime() . (function_exists("getmypid") ? getmypid() : "") . uniqid(rand(), TRUE);

This won't go into upstream phpass for awhile due to it's PHP3 dependency, but we can add it if we like.

03/20/08 19:46:51 changed by westi

  • cc set to westi.

03/20/08 19:56:37 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.

(In [7421]) Use uniqid if getmypid is disabled. Props Solar Designer. fixes #6293