New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use strong random number/string generators inside generatePassword() #15740
Comments
Unfortunately for MODX 2.x we're still in the old php5 days, but this certainly could and should be improved in 3.x. I wouldn't go as far as saying this particular code is vulnerable because it's not cryptographically secure random but a good improvement nonetheless. |
Could it be possible at least to add some special chars in the allowable_characters? I mean here:
Alessandro |
It's probably worth creating a field in the "User" section where you can specify valid characters for the password, or a field in "System Settings". |
yes it would be more scalable, my suggestion was for a quick improve for modx 2, I didn't undestand if it's going to be changed in modx 3, |
Feature request
Summary
Looking for a native solution to create random strings, I stumbled upon the generate Password function today.
I've noticed that the "randomness" of the generated Password relies on mt_rand(), a "Pseudo-Random Number Generator". I guess this is a pice of code that has been active from the old php5 days. PRNG are considered to be a "weak" nowadays.
Basing a random number on a timestamp using srand() is also considered to be a potential problem.
Why is it needed?
Random passwords should be random by todays standards.
I'm aware that its very unlikely that the login mechanism might be compromised by this "vulnerabilty", but I like to point out that a modx developer might make use of this function to generate a "secure" and "random" strings for other purposes.
Suggested solution(s)
In php 7 "Cryptographically Secure Pseudo-Random NUMBER/STRING Generators" are available. These methods are considered to be "strong":
PHP 5.x polyfill for random_bytes() and random_int()
SIDENOTE:
Discussion
Cheers patrick
The text was updated successfully, but these errors were encountered: