Opened 9 years ago
Last modified 4 years ago
#34297 reviewing enhancement
Passwords containing ' or " via wp_set_password() break login via wp-login.php
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Users | Keywords: | needs-patch needs-docs the-road-to-magic-quotes-sanity |
Focuses: | docs | Cc: |
Description
We are using custom plugins to have the user reset their passwords. Internally all that is done boils down to the following code:
wp_set_password('Test"123', 1); // alternative test wp_update_user([ 'ID' => 1, 'user_password' => 'Test"123' ]);
Doing this the password gets successfully changed inside the database.
Trying to login on wp-login.php now with the new password results in: "ERROR: The password you entered for the username admin is incorrect"
Login however still works using wp_signon().
I don't know what's going on there or where the differences are but surely this can't be intended behavior that we're not supposed to set passwords using ' or " via wp_set_passworod(), can it?
Attachments (2)
Change History (25)
#2
@
9 years ago
I wonder what I did wrong then when I manually escaped it myself :S
I can confirm that this solves the issue, so as you mentioned this is simply a documentation issue and not a bug itself.
wp_signon()
and wp_set_password()
and wp_update_user()
would require this information. Potentionally more but those are the ones that I came into contact with now.
Thank you for the super quick response!
#3
@
9 years ago
- Milestone Awaiting Review deleted
- Resolution set to invalid
- Status changed from new to closed
#4
@
9 years ago
- Keywords needs-docs added
- Resolution invalid deleted
- Status changed from closed to reopened
#6
@
9 years ago
Your only other choice is:
/** * Strip the slashes from Cookies */ function _fix_cookies() { $_COOKIE = stripslashes_deep( $_COOKIE ); } add_action( 'sanitize_comment_cookies', '_fix_cookies' );
I almost threw my computer out the window 4 years ago when I found this awful mess.
This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.
9 years ago
#12
@
9 years ago
- Keywords good-first-bug added
Two steps to write the patch:
- Look through the functions that expect a password, and determine (maybe through trial and error) which of them expect passwords to be slashed.
- Modify the docs accordingly.
#14
@
9 years ago
Hello @manuakasam and @ocean90, wouldn't it be a nice idea to, instead of mentioning the wp_slash()
in the docs for wp_signon()
, wp_set_password()
and wp_update_user()
, write a check for these quotes in the plain text password and if so, wrap the wp_slash()
function around the plain text password in core ourselves so that we can take away the need for developers to really needing to read that part of the documentation?
#17
@
8 years ago
Apologies, the red and green colored code block in the previous patch should be reversed.
A revised version was uploaded.
#18
@
8 years ago
- Keywords good-first-bug removed
- Milestone changed from Future Release to 4.7
- Owner set to johnbillion
- Status changed from assigned to reviewing
- Type changed from defect (bug) to enhancement
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#22
@
8 years ago
- Component changed from Login and Registration to Users
- Keywords needs-patch needs-docs the-road-to-magic-quotes-sanity added; has-patch removed
- Milestone changed from 4.7 to Future Release
Punting this because, incredibly, it's actually difficult to tell which fields in these functions need slashing and which don't. Needs someone to spend some time going through the call stack.
Hello @manuakasam, welcome to Trac!
Previously: #26573, #24367
It looks like you have to slash the passwords via
wp_slash()
first. I think we should add a note for this to all the password functions.