Make WordPress Core

Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#23494 closed defect (bug) (fixed)

impossible to log in with password containing leading or trailing spaces

Reported by: mich1 Owned by: westi
Priority: high Milestone: 3.6
Component: Users Version: 2.5
Severity: major Keywords: needs-patch 2nd-opinion
Cc: Focuses:

Description (last modified by ocean90)

It is possible to set the password to something containing trailing or leading space characters (e.g. a single space), but the login dialog trims trailing or leading whitespaces. This makes it impossible to log in again.

It seems to me that the behavior of the login dialog is intended (see #15439), thus passwords with leading or trailing spaces should not be allowed.

Steps to reproduce (best with a new user account):

  • log in
  • change your password to something with leading or trailing spaces
  • log out
  • try to log in again. If you set your password to spaces only you will get the same error message as if you entered no password, otherwise you will get the message that the password you entered was wrong.

Note: When trying to create a user with leading or trailing spaces that differs only by those spaces from an existing user name there is an error message that the user name already exists, which is not totally correct. It still might be helpful though.

Change History (11)

#1 @ocean90
14 years ago

  • Description modified (diff)

#2 @SergeyBiryukov
14 years ago

  • Version 3.5.1 → 2.5

trim() for password was added in [6643], moved to wp_authenticate() in [10437].

#3 @westi
14 years ago

  • Component General → Users
  • Keywords needs-patch 2nd-opinion added
  • Milestone Awaiting Review → 3.6
  • Owner set to westi
  • Priority normal → high
  • Severity normal → major
  • Status new → accepted

This is kind of sucky because we now have this easy to get into situation:

# Change password to a passphrase with a space on the end or begining
# Lock yourself out and have to go through the password reset dance.

Currently all these users are "locked out" so we don't have to support their passwords but we should either support them by removing the trim in wp_authenticate() or add a trim in wp_set_password to match the other trim so that users can't get themselves into this confusing mess.

Marking for 3.6 as I think we should resolve this sooner rather than later as the use of pass phrases is becoming more common and I am seeing more reports of this issue.

#4 @westi
14 years ago

In 1250/tests:

Pluggable Functions: Add some tests for passwords with trailing and leading spaces.

See #23494.

#5 @westi
14 years ago

  • Resolution → fixed
  • Status accepted → closed

In 23814:

Pluggable Auth: When setting new passwords for users trim any leading or trailing space to match what we do when we test passwords.

Fixes #23494

#6 follow-up: @nacin
14 years ago

Should we do both in wp_authenticate()? With and without trim? To prevent current bad passwords from not working? Or, since they never worked, is this less of an issue?

#7 in reply to: ↑ 6 @westi
14 years ago

Replying to nacin:

Should we do both in wp_authenticate()? With and without trim? To prevent current bad passwords from not working? Or, since they never worked, is this less of an issue?

"Currently all these users are "locked out" so we don't have to support their passwords" was the conclusion I came to.

The passwords never worked so I decided it was better to not confuse the issue by trying to support them.

#8 @rpattillo
13 years ago

  • Resolution fixed
  • Status closed → reopened

This does not appear to be completely fixed in 3.6. The error can still be reproduced by a user changing his or her password via Edit Profile.

Not all changes to a user's password go through wp_set_password, which is where the trim() call was added before passing the plain text password to wp_hash_password. In wp-includes/user.php, inside both wp_insert_user() and wp_update_user(), the plain text value is sent to wp_hash_password without being sent through trim() first.

It appears to me that the call to trim() should be moved to wp_hash_password() to insure it is used consistently:

return $wp_hasher->HashPassword($password);

modified to be:

return $wp_hasher->HashPassword( trim( $password ) );

#9 @helen
13 years ago

  • Resolution → fixed
  • Status reopened → closed

This ticket was closed on a completed milestone. Please open a new one.

#11 @nacin
13 years ago

In 25709:

Move the trim() from wp_set_password() to inside wp_hash_password().

props rpattillo, joehoyle.
fixes #24973. see #23494.

Note: See TracTickets for help on using tickets.