search

slackero / phpwcms

Flexible, fast, powerful, customer, developer friendly web content management system and cms framework
http://www.phpwcms.org
GNU General Public License v2.0
92 stars 45 forks source link

Sending hashed md5 passwords for authentication #214

Closed herenow closed 7 years ago

herenow commented 7 years ago

Hi,

I've noticed that we are hashing to md5 the passwords before sending them for authentication, here:

https://github.com/slackero/phpwcms/blob/master/login.php#L131

We then compare the sent hashed password w/ the stored hashed password.

Is there any reason for this?

I think the good side of this, is that at least, if the site doesn't have https, the password won't be sent over plaintext.

The bad side of this, is that if our database gets compromised, the attacker can manipulate the request body, and use the leaked md5 passwords to authenticate.

IMO, the latter is worse, since the former can we fixed with SSL, and SSL is widely available nowadays.

I guess we could also just salt the stored password, but that would be a breaking change.

Thanks.

slackero commented 7 years ago

Yes the password needs to be salted and better hashing too, but it's better to send hashed passwords instead of plaintext as normal. So it's by far still an unknown password and only phpwcms might be broken then not all other accounts using the same password.

It's on my list and is also no breaking change.