WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#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)

comment:1 ryan6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [7836]) Add arg to make special chars optional when generating passwords. fixes #6842 for trunk

comment:2 ryan6 years ago

(In [7837]) Add arg to make special chars optional when generating passwords. fixes #6842 for 2.5

comment:3 thee176 years ago

  • Milestone 2.7 deleted

comment:4 Otto426 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.

comment:5 Otto426 years ago

Additional.

Report on the forum is that the given fix I posted before works:
http://wordpress.org/support/topic/172820

comment:6 ryan6 years ago

Or, we can blank all user_activation_keys during upgrade to 2.5.2.

comment:7 Otto426 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.

comment:8 sbtalk6 years ago

  • Cc sbtalk.com.cn@… added

comment:9 follow-up: jimmysoho6 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.

comment:10 in reply to: ↑ 9 DD326 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.

comment:11 ryan6 years ago

  • Owner changed from anonymous to ryan
  • Status changed from reopened to new

comment:12 pishmishy6 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.

comment:13 ryan6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [7936]) Clear out bad activation keys from 2.5.1. fixes #6842 for trunk

comment:14 ryan6 years ago

(In [7937]) Clear out bad activation keys from 2.5.1. fixes #6842 for 2.5

comment:15 ryan6 years ago

  • Milestone changed from 2.5.2 to 2.9

Milestone 2.5.2 deleted

comment:16 westi6 years ago

  • Milestone changed from 2.9 to 2.6
Note: See TracTickets for help on using tickets.