No validation done on login/setNewPassword prior to calling the setNewPassword method on Login model

[enygma]

If an attacker hits the login/setNewPassword endpoint directly with a POST request containing the required fields (the ones checked for inside of the Login model) they could, in theory, change any user's password. You're passing in the username and hash data for validation but you're not validating that against anything.

My advice would be to not pass in the username at all and instead work up a hash that uses that and some other pieces of information to generate a unique hash on the previous form (maybe put that into the session like a CSRF token) and validate that.

[panique]

I highly doubt that. Don't have the time to check back, but I'll comment on that when there's time. Please don't get me wrong, but in 99% of "bug reports" it's not a bug, just a misunderstanding of the code. Are you 100% sure ? Can you describe an concrete case ?

[panique]

In login->setNewPassword the query clearly changes the password for the username/reset-hash-combination in line 1025+. It's impossible to change a password without having the password-(reset-)hash. Can you please re-check this ?

In line 1022 we use the PHP'S password hashing API to generate a new password hash, so we also have some kind of throttling here (to prevent people sending millions of requests to the app so easily).

Can you please re-check ?

[enygma]

Having that hash in the POST request isn't the same thing as validating it.

Scenario:

1. call /login/setNewPassword directly (no previous form submission) with all required fields
2. Login model's setNewPassword is called and data is checked for existence
3. The submitted "user_password_new" value is used to create the hash along with a cost (which you've defaulted to 10), easily reproducible by the attacker
4. The UPDATE statement is called with the data the user submitted and the password is changed.

Remember, HTTP is stateless...there's no requirement that they went through your password reset flow and only have valid information. The key here is that they went directly to the setNewPassword action in the Login controller and are able to specify the user and password they want.

[panique]

I think you're mixing up "the hash". There are several hashes in this method. It's not possible to change anything unless the provide the exact password-reset-hash. No way around this. But regardless of that, checking the reset-hash within the query is not the best way to do it, there should be a throtlting pre-query.

Side-fact: Brute-forcing this on an not-professional server would take months while killing the server, too. So it's not a really realistic case. It would be 1000000x easier to brute-force most md5-hashed sites in the world.

Feel free to correct me here, maybe you are right, but please give an prooveable reproduceable example. Feel free to perform this on the demo installation.

[enygma]

Bah, you're correct...I missed the line in the SQL looking for the right hash (user_password_reset_hash = :user_password_reset_hash) in the SQL statement. My apologies!

[panique]

No prob, no need to apologize! It's always good to have feedback!