View Issue Details

IDProjectCategoryView StatusLast Update
0000556XMB1Bugspublic2020-10-20 18:17
Reportersimonw Assigned Tomiqrogroove  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.9.8 SP2 
Target Version1.9.12Fixed in Version1.9.12 
Summary0000556: Insufficient entropy on password generation
DescriptionWhen an account is reset the function mt-srand is called using the output of the microtime() function as a seed.
This function is also used elsewhere, e.g. CSRF, but the exploitation is unclear, I would change all instants of mt-rand/mt-srand when a solution is agreed.

This means that although it generates a 13 character password with 62 possible characters in each place.
It only can 1,000,000 possible passwords.

I wrote a trivial PHP function to generate all possible passwords which takes 12 seconds to run.

I reset my test server's admin password, and a single threaded brute force attack found the 145245 password was needed in 1h 4m 47 seconds allowing me access to the admin account knowing only account name and email (the email is used in sending out messages from the forum).

The attack could be speeded up, since the attacker can know the server time from HTTP headers, and can estimate the value of the microtime() to within a few milliseconds. Because of the problematic way XMB does authentication the password needs to be really strong, since to prevent brute force guessing all authenticated actions would need special controls.

Worse case is under 8 hours at this rate of attack.
Correctly implemented a 62^13 character password should take ~1000,000,000 times the estimated age of the Universe at this rate of attack.

I looked at replace mt-srand(double(microtime()*1000000)) with mt-srand() as the PHP manual page suggests, but the mt-srand() built-in initialisation function is mathematically broken as well.

PHP 7 has already reverted one fix for mt-srand() as it breaks backward compatibility (sigh).

I'd strongly recommend abandoning mt-srand and related randomisation functions ASAP.

The PHP function "random_int" can be used in place of "mt-rand()", and implementation exists for pre-7.0 users https://www.php.net/manual/en/function.random-int.php

Steps To ReproduceReset a password, compare to password to the list generated from:

<?php

/* Generate all 1,000,000 possible passwords the XMB forum reset can generate */

$chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz";
$get = strlen($chars) - 1;

for ($seed = 0 ; $seed < 1000000 ; $seed++){
 mt_srand( $seed );
 $password = '';
  for($i = 0; $i < 13; $i++) {
   $password .= $chars[mt_rand(0, $get)];
  }
 print $password."\n";
}
?>
Additional InformationI tested the modified code snippet on Debian GNU/Linux 10 on AMD processor, this created 1000,000 passwords which were unique, strong, and none of them were in the list generated by the snippet above. The list was put through the NIST randomness tests offered by Burp Suite Community Edition and passed, although there is indication of use of pseudorandom number generators, which is expected if it is sourcing /dev/urandom or similar at some points in the process.

$chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz";
$get = strlen($chars) - 1;

for ($seed = 0 ; $seed < 1000000 ; $seed++){
 $password = '';
  for($i = 0; $i < 13; $i++) {
   $password .= $chars[random_int(0, $get)];
  }
 print $password."\n";
}
Tagspassword
MySQL Version
PHP Version
Web Server
Browser
Flags
Original Reporter
SVN Revision2798

Relationships

related to 0000001 closedmiqrogroove Full Session Handling 
related to 0000559 closedmiqrogroove Increase Required PHP Version to 7.0 

Activities

miqrogroove

2020-08-17 20:10

administrator   ~0000385

This would likely affect all versions. I also think there is a larger issue if XMB is allowing 145,245 login attempts per hour. That's something like 40 hits per second? Maybe I can add a rate limit but I don't recall if the attempt date is stored at all in this schema.

simonw

2020-08-18 02:02

reporter   ~0000386

Rate limiting wouldn't be effective on login alone, as you use the password MD5 as an effective session token, an attacker can thus perform any authenticated action as a check on whether a word is the user's current password, you'd need the rate limit check on login, and on all actions that check authentication.

I think rate limiting would be good as it stops general password guessing too, but you probably want to also ensure the generation of randomness is correct too, since the attacker can improve the attack by using the HTTP headers to work out the server offset from their clock time, and then measure the time of the HTTP reset request, and know the server microtime() value to within a much narrower range than my proof of concept.

miqrogroove

2020-08-18 15:36

administrator   ~0000387

This is all handled by a single function since v1.9.10. Unfortunately, there was never support for rate limiting in the database schema. And worse, we are still at the PHP 4.3.0 support level. So, an overhaul of the XMB session handler is going to require major changes.

miqrogroove

2020-08-28 05:39

administrator   ~0000393

The mt_srand() calls have been replaced in the trunk version of XMB. There was also an old ticket regarding the other session handling problems. That older ticket will be used to track those issues.

Issue History

Date Modified Username Field Change
2020-08-15 05:36 simonw New Issue
2020-08-15 05:36 simonw Tag Attached: password
2020-08-17 20:10 miqrogroove Note Added: 0000385
2020-08-18 02:02 simonw Note Added: 0000386
2020-08-18 15:36 miqrogroove Note Added: 0000387
2020-08-22 14:34 miqrogroove Status new => confirmed
2020-08-22 14:34 miqrogroove Product Version 1.9.11.15 => 1.9.8 SP2
2020-08-22 14:34 miqrogroove Target Version => 1.9.12
2020-08-22 14:34 miqrogroove Flags => Schema Updates
2020-08-22 14:44 miqrogroove Relationship added related to 0000001
2020-08-23 08:48 miqrogroove Relationship added related to 0000559
2020-08-28 05:39 miqrogroove Assigned To => miqrogroove
2020-08-28 05:39 miqrogroove Status confirmed => resolved
2020-08-28 05:39 miqrogroove Resolution open => fixed
2020-08-28 05:39 miqrogroove Fixed in Version => 1.9.12
2020-08-28 05:39 miqrogroove Flags Schema Updates =>
2020-08-28 05:39 miqrogroove SVN Revision => 2798
2020-08-28 05:39 miqrogroove Note Added: 0000393
2020-10-20 18:17 miqrogroove Status resolved => closed