mimecorg / webissues

WebIssues team collaboration system.
https://webissues.mimec.org/
GNU Affero General Public License v3.0
59 stars 14 forks source link

Why is MD5 used for hashing Password? #62

Closed TAINCER closed 1 year ago

TAINCER commented 1 year ago

I've noticed that a custom Class was written for generating the Hash for Passwords, which salts the Password with MD5. Ref: https://github.com/mimecorg/webissues/blob/ad693d0dee01456a962e03251ff49df98429f58e/system/core/passwordhash.inc.php#L125-L133

Salted MD5 is not fitted for hashing Password and is widely not regarded safe for this purpose. (Ref)

Is there a reason not to use the Build-In functions from PHP that comes with way better Hashing Alogorymths, namely: password_hash for creating and password_verify, both available since PHP5.

As it stands now, I see this as a massive Security Vulnerability. Although this could be very easily migrated to the native PHP functions by replacing the body of the relative method in the PasswordHash class since the signature is very similar, if not identical.

mimecorg commented 1 year ago

This custom class is based and fully compatible with phpass, which is used by major PHP applications, including WordPress. Although salted MD5 is not secure, the phpass algorithm uses password stretching to make generating the hash much more computationally expensive, and therefore more secure.

TAINCER commented 1 year ago

Stretching the password may be more secure, but is still not comparable with bcrypt, which is used in password_hash by default.

Also given that the authors from phpass [say own their Website](https://www.openwall.com/phpass/#:~:text=At%20this%20time%2C%20if%20your%20new%20project%20can%20afford%20to%20require%20PHP%205.5%2B%2C%20which%20it%20should%2C%20please%20use%20PHP%27s%20native%20password_hash()%20/%20password_verify()%20API%20instead%20of%20phpass.) to not use it unless the Project requires < PHP 5.5, which is not the case here.

At this time, if your new project can afford to require PHP 5.5+, which it should, please use PHP's native password_hash() / password_verify() API instead of phpass.

I don't see the point in not using the native functions with a better Hashing Algorithm. The only downside would be, that Users have to re-enter/change their password one time. Given the gain in Security is a no-brainer in my opinion. Although, this could be mitigated by using both - phpass and the native functions - for a short time. Deprecating phpass, and only using it to check passwords, but not store new ones. When logging in, if the phpass check method returns false, also check the password_verify function. Also, notify the Users in the client to re-enter their password to use the better hashing algorithm. And eventually, remove phpass completely in a few releases.

I also don't know, if WordPress is the best Password Security Benchmark for this. They also don't use the Vanilla phpass library. They modified it to use 8192 iterations when hashing. Frameworks like Drupal 7 changed phpass to use SHA-512. (Ref)

It is not hard to crack a Password generated with phpass, there are multiple tools, like hashcat, that have support for this exact hash type. Cracking simple passwords in 1-10 Seconds, more complex in a few minutes. No Hashing Alogrymptm is perfect, but this is just too fast and renders the hashing itself obsolete.

I would fork this anyway and implement it in such a way since we plan on using this tool. I can open a PR regarding this if wanted.

mimecorg commented 1 year ago

You're welcome to open a PR. I have nothing against using password_hash as long as the current hashes continue to work - they can be updated automatically when the user logs in.

TAINCER commented 1 year ago

I can only think of two solutions for this problem since it's not possible to rehash the old md5 hash with bcrypt:

In both cases, only the md5 methods for checking the password would still be present. Everything with generating new hashes would be handled by password_hash with bcrypt.

mimecorg commented 1 year ago

They don't have to enter a new password. When a user logs in and the password is validated using phpass, it can be automatically hashed using password_hash and updated in the database.