This is more a usability issue than a bug: When the user tries to change both his email address and his password at the same time, the confirmation link will be invalid, as the hash code still corresponds to the old password.
The easiest way to prevent this is probably to throw a form error if the new password field is not blank.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | email_confirm-change-email-and-password_1710608-7.patch | 671 bytes | alexandrdnlv |
| #6 | email_confirm-change-email-and-password_1710608-6.patch | 664 bytes | klidifia |
Comments
Comment #1
ropic commentedconfirmed +1
Comment #2
ropic commentedline 420 must be updated:
$pass = isset($edit['pass']) ? md5($edit['pass']) : $user->pass;to
$pass = isset($edit['pass']) ? $edit['pass'] : $user->pass;$edit['pass'] is encrypted
Comment #3
jaydub commentedComment #4
klidifia commentedAlso causes an issue if someone changes their email address when they've logged in for the first time (where they also have to set their password).
Comment #5
klidifia commentedFor current stable
Comment #6
klidifia commentedComment #7
gregglesThis change looks good. It seems like at least a "normal" priority.
Comment #8
gregglesOops, better status as well.
Comment #9
emsearcy commentedI think patch #6 may still have an issue with systems that don't use the password supplied by the user, such as simple_ldap, which writes the user-provided password to LDAP but saves a random password into the user database. In any case, I'd suggest that the password isn't needed at all, but rather the user ID should be included. In fact, all the parameters in the URL (aside from the hash itself) should be included into the hash, otherwise a user can change the timestamp arbitrarily to use old links. I've just created a public issue about this (#2463481) which should also fix this issue by no longer including the password in the hash (similar to LoginToboggan's email confirmation links).
Comment #10
alexandrdnlv commentedA little bit updated patch for updated function.
Comment #11
gregglesI agree with emsearcy in #9 about just removing the password from the hash. I think this would be fixed by #2463481: Expirations can be bypassed and username enumeration, so closing it out as duplicate of that.