#24973 closed defect (bug) (fixed)
Impossible to login with passwords that contain trailing or leading spaces
Reported by: | rpattillo | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.7 | Priority: | normal |
Severity: | normal | Version: | 3.6 |
Component: | Users | Keywords: | has-patch |
Focuses: | Cc: |
Description
It is possible to set a user's password to a string that includes leading or trailing spaces. The spaces will be hashed with the password as it is saved. During login attempts, the spaces will be trimmed before hashing, making it impossible to log in again.
To reproduce:
1] Log in.
2] Navigate to /wp-admin/profile.php to edit your profile.
3] In the new password and confirmation fields, enter any password followed by trailing spaces.
4] Logout
5] Try to log in using the new password with trailing spaces and without.
What is expected:
The password without trailing spaces should work. This would be in line with what happens if the password is changed via the lost password system, and the new password has trailing or leading spaces.
What actually happens:
Neither the password without trailing spaces nor the password with the same trailing spaces will work. It will be impossible for the user to login until the password is reset again.
This is similar to ticket #23494. The resolution there was to trim the password in wp_set_password() before passing it to wp_hash_password(). However not all methods of changing the password go through wp_set_password().
Attachments (3)
Change History (11)
#2
@
11 years ago
- Keywords needs-unit-tests added
trim-passwords.diff could very much benefit from unit tests.
#3
@
11 years ago
- Keywords needs-unit-tests removed
24973.diff refreshes the current patch against develop.svn and adds unit tests for testing a trim of the password in wp_hash_password()
.
#4
@
11 years ago
@jeremyfelt: looks like to beat me to it! Though mine tests new lines and vertical tabs :P
#5
@
11 years ago
Better tests indeed! It looks like some leftovers from another ticket made their way into trim-password-2.diff. Nothing big, only some docs.
#6
@
11 years ago
I cleaned the patch by overwriting it, are you maybe you were looking at it before I updated the patch?
#7
@
11 years ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 25709:
#8
@
11 years ago
WordPress is trim()
ing passwords to make a better UX. Going down the same route, should WordPress also strtolower()
passwords all the time, in case users accidentally have caps lock on? Should WordPress remove duplicate consecutive characters, in case the user held a key down too long?
Both trim()
and strtolower()
sacrifice password entropy for UX. You could easily argue that the UX gains from this ticket are worth the reduction in security, but the scary thing is that the security implications don't seem to have been considered! (At least in this ticket and from a quick Google, apologies if this was discussed elsewhere).
Is WordPress' modification of users' passwords documented anywhere?
SVN diff file, adding trim() to wp_hash_password() and removing it from wp_set_password()