Password reset links produce invalid keys

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

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

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.

Additional.

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

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

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.

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.

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.

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 [7936]) Clear out bad activation keys from 2.5.1. fixes #6842 for trunk

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