xiaomlove / nexusphp

A private tracker application base on NexusPHP
https://nexusphp.org
GNU General Public License v2.0
901 stars 185 forks source link

Insecure passwords thanks to a md5? #280

Closed slrslr closed 1 month ago

slrslr commented 1 month ago

https://github.com/xiaomlove/nexusphp/blob/4d865384ddc43695950e17227fb29ed749730284/include/functions.php#L2997

https://github.com/xiaomlove/nexusphp/blob/4d865384ddc43695950e17227fb29ed749730284/public/takesignup.php#L151

Asking ChatGPT about this method of generating password results in:

It is using a combination of a randomly generated secret and the user input password to create a hash for password storage. While this method may provide some level of security, it is important to note that using MD5 for hashing passwords is considered insecure due to its vulnerability to hash collisions and brute force attacks.

You can consider the following improvements:

  1. Upgrade the hashing algorithm: Replace the use of MD5 with a more secure hashing algorithm such as bcrypt or Argon2. These algorithms are specifically designed for password hashing and offer better security against various attacks.

  2. Add salt to the hashing process: Include a unique salt value for each password hash to prevent rainbow table attacks. This salt should be stored securely and can be generated using a cryptographically secure random number generator.

  3. Implement password stretching: Apply multiple iterations of the hashing algorithm to slow down brute force attacks. This technique, known as key stretching, increases the computational cost of generating password hashes.

By implementing these enhancements, you can significantly improve the security of the password storage in your PHP script while maintaining compatibility with existing passwords and comparison methods. It is recommended to carefully test these changes in a controlled environment before deploying them to production.

md5: I have confirmed from other sources that md5 is bad practice nowadays. bcrypt: "Use scrypt when you can; bcrypt if you cannot." https://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords salt: https://stackoverflow.com/questions/1645161/salt-generation-and-open-source-software/1645190

xiaomlove commented 1 month ago

This insecurity exists only in theory, with a CAPTCHA and a limit on the number of failures, and a low likelihood of brute force cracking. Changing this piece requires all users to reset their passwords, which has a large impact. We will consider changing this in the next major release.

slrslr commented 1 month ago

You are right that the brute-force login protection seems good. But I have not written this issue with a brute-force login concern in mind, but being concerned about the database leak of the members data (passwords in this case).

As said, currently used crypto per my understanding is insecure and a bad practice, see my initial post please. As such i think that this issue should remain open or being noted as a "milestone" for the next major release.