From 00d862bb44930c64a969ebe69223c5cf85b9fa0f Mon Sep 17 00:00:00 2001 From: frankfarmer Date: Sat, 10 Oct 2015 07:49:56 -0700 Subject: [PATCH] Correct implemenation of User::randomPassword By default, randomPassword is meant to generate a 10 character random password. This is achieved by the generation of a random 10 digit base32 int. The return value SHOULD ideally be evenly distributed between 0000000000 and vvvvvvvvvv However, the 2012 implementation of this function actually returns a value between 0 and 7vvvvvvvvv. The old implementation can easily be observed generating passwords as short as 7 characters when tested, and can theoretically generate a password with just a single character. $sourceLength is set to 12.5 when attempting to generate a default 10 char password in the 2012 implementation (which results in only 12 hex chars returned). 13 hex digits are necessary to get a full 0 - vvvvvvvvvv distribution (the full space of 10 digit base 32 ints). This actually gives us a few bits too many, so we take only the least significant bits, giving us a full, even distribution of the desired keyspace. Even then, we might theoretically get back a number with only 1-9 digits; we then zero-pad these to get a 10 character representation. --- includes/User.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/includes/User.php b/includes/User.php index 75649a7..07cf732 100644 --- a/includes/User.php +++ b/includes/User.php @@ -1030,11 +1030,18 @@ public static function randomPassword() { // stopping at a minimum of 10 chars. $length = max( 10, $wgMinimalPasswordLength ); // Multiply by 1.25 to get the number of hex characters we need - $length = $length * 1.25; + $sourceLength = ceil($length * 1.25); // Generate random hex chars - $hex = MWCryptRand::generateHex( $length ); + $hex = MWCryptRand::generateHex( $sourceLength ); // Convert from base 16 to base 32 to get a proper password like string - return wfBaseConvert( $hex, 16, 32 ); + $base32 = wfBaseConvert( $hex, 16, 32 ); + // Take the $length least significant bytes of the string (they should be evenly distributed) + // -- the highest byte, on the other hand, may not be evenly distributed from 0-v + $lsb = substr($base32, -1 * $length); + // Theoretically the number that has been returned may be as low as 0 (with just one char); + // we should pad our base32 number out to return a password with $length digits + $padded = str_pad($lsb, $length, "0", STR_PAD_LEFT); + return $padded; } /**