WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 22 months ago

#23494 closed defect (bug) (fixed)

impossible to log in with password containing leading or trailing spaces

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

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)

comment:1 @ocean902 years ago

  • Description modified (diff)

comment:2 @SergeyBiryukov2 years ago

  • Version changed from 3.5.1 to 2.5

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

comment:3 @westi2 years ago

  • Component changed from General to Users
  • Keywords needs-patch 2nd-opinion added
  • Milestone changed from Awaiting Review to 3.6
  • Owner set to westi
  • Priority changed from normal to high
  • Severity changed from normal to major
  • Status changed from new to 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.

comment:4 @westi2 years ago

In 1250/tests:

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

See #23494.

comment:5 @westi2 years ago

  • Resolution set to fixed
  • Status changed from accepted to 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

comment:6 follow-up: @nacin2 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?

comment:7 in reply to: ↑ 6 @westi2 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.

comment:8 @rpattillo2 years ago

  • Resolution fixed deleted
  • Status changed from closed to 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 ) );

comment:9 @helen2 years ago

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

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

comment:11 @nacin22 months 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.