WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#6293 closed defect (bug) (fixed)

phpass should use uniqid(), not getmypid()

Reported by: tellyworth Owned by:
Milestone: 2.5 Priority: normal
Severity: blocker Version: 2.5
Component: General Keywords: has-patch
Focuses: Cc:

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 (1)

phpass-uniqid-r7392.patch (444 bytes) - added by tellyworth 6 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Viper007Bond6 years ago

  • Severity changed from normal to blocker
  • Version set to 2.5

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.

comment:2 Viper007Bond6 years ago

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.

comment:3 DD326 years ago

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.

comment:4 Viper007Bond6 years ago

Yeah, I know. :)

comment:5 ryan6 years ago

Solar Designer, author of phpass, is investigating.

comment:7 ryan6 years ago

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.

comment:8 westi6 years ago

  • Cc westi added

comment:9 ryan6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.