#6842 closed defect (bug) (fixed)
Password reset links produce invalid keys
Reported by: | MtDewVirus | Owned by: | ryan |
---|---|---|---|
Milestone: | 2.6 | Priority: | normal |
Severity: | normal | Version: | 2.5.1 |
Component: | Security | Keywords: | |
Focuses: | Cc: |
Description
When using /wp-login.php?action=lostpassword the password reset link received in the email does not work. When clicking on the link, you get "Sorry, that key does not appear to be valid."
Also, some of the characters used in the key aren't treated as part of a link in email (Gmail as an example).
Example: http://blog.com/wp-login.php?action=rp&key=yG#S^w4U&QY(
Only http://blog.com/wp-login.php?action=rp&key=yG#S was treated as a link in Gmail and the rest was plain text.
Tested on r7835
Change History (16)
#4
@
17 years ago
- Milestone set to 2.5.2
- Resolution fixed deleted
- Status changed from closed to reopened
- Version changed from 2.6 to 2.5.1
This fix is incomplete. If the user_activation_key field contains any of these special characters, then a valid key will not be generated. So people who upgrade from the broken 2.5.1 to a fixed 2.5.2 will need to manually clear their activation keys, or the code needs to be altered to recognize broken keys and replace them anyway.
Suggestion: Code in wp-login.php:
$key = $wpdb->get_var($wpdb->prepare("SELECT user_activation_key FROM $wpdb->users WHERE user_login = %s", $user_login)); if ( empty($key) ) {
Change the if check to this:
if ( empty($key) || preg_match('/[^a-z0-9]/i',$key) != 0) {
Which basically says that if it finds any characters that are not a-z0-9, then it'll regenerate.
#5
@
17 years ago
Additional.
Report on the forum is that the given fix I posted before works:
http://wordpress.org/support/topic/172820
#7
@
17 years ago
You can do that, but I like my fix better. A sanity check on the value stored in the DB is not a bad idea at that point in the code. If bad data gets into that field in some other way, this sanity check will catch it.
#9
follow-up:
↓ 10
@
17 years ago
Just get rid of the "special chars" in wp_generate_password, they should never be part of any password key. As the original description of this ticket explains, it leads to URL links that aren't valid URLs. Another solution would be to URL decode the key before sending it out, but that leads to other problems as you may imagine.
#10
in reply to:
↑ 9
@
17 years ago
Replying to jimmysoho:
Just get rid of the "special chars" in wp_generate_password, they should never be part of any password key.
Well, Now that they're optional its not a problem.
Using special chars greatly increases the complexity of the generated string, Which is of great use when being used internally for things, eg. as a seed for a password hashing function.
I think Blanking the User activation keys on upgrade to 2.5.2 as ryan said would be the best method, Simply wipe them out, Sanitizing the value shouldnt be needed as hopefully, assuming the value stays valid in the future.
#12
@
17 years ago
This shouldn't have crept in with Changeset 6643. I purposely left the key generation as
substr( md5( uniqid( microtime() ) ), 0, 8);
to avoid this specific problem. Never mind, I'll learn to comment my code properly next time.
(In [7836]) Add arg to make special chars optional when generating passwords. fixes #6842 for trunk