amtgard / ORK3

Version 3 of the Online Record Keeper
Other
23 stars 11 forks source link

MD5 used for passwords #204

Closed zellfaze-zz closed 5 years ago

zellfaze-zz commented 5 years ago

MD5 hashes are being used for passwords. This is a really bad idea, even if they are salted. As of PHP5 there is actually a password_hash() function. I would recommend using this as it uses crypt() as its backend, which is actually meant for use with passwords.

The issue with MD5 is that it is very cheap to calculate an MD5 hash. This means passwords will be pretty straightforward to crack.

esdraelon commented 5 years ago

Fixed in issue/remove-leaking-password

The passwords were being used as de facto sources of psuedorandom information in token creation. Although the tokens are not leaked directly to the browser, they are stored in archives. Most of them were seeded with calls to microtime(), so the collision attack is much less likely to give you the actual password. I won't say a million times less than the already low chance of getting the correct password, since the issue with MD5 is collision state-space. But a lot less.

In order to log in, having an md5() collision doesn't buy you much since the actual passwords are stored and compared against 5000 rounds of SHA-512 cyphertext with a salt.