Make WordPress Core

Opened 3 years ago

Last modified 2 days ago

#52529 new defect (bug)

Non-slashed $old_user_data->user_email in wp_insert_user causes user_activation_key to be unset

Reported by: emirpprime's profile emirpprime Owned by:
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

WordPress now allows apostrophes in email addresses, but the forgot password process for these users fails.

$data['user_activation_key'] in wp_insert_user() get's cleared when the $user_email !== $old_user_data->user_email comparison fails.
This is because $old_user_data is the "clean" data from the database, however wp_update_user() calls add_magic_quotes() on the user data it passes, causing apostrophes in email addresses to be slashed.

Process fails silently as there is no error when $data['user_activation_key'] is cleared in this flow, even though it is required for the forgot password system to function.

To replicate - register a user with an apostrophe in their email, use the lost password system from wp-login.php. An email will be sent/received, but the reset link is deemed invalid as there is no user_activation_key in the db.

Patch (about to be) attached to slash email only in $old_user_data. While this fixes the bug, it does raise the question of whether get_password_reset_key() should validate that the key is created directly as wp_update_user() errors don't give the granularity to tell us.

Attachments (1)

52529.patch (912 bytes) - added by emirpprime 3 years ago.

Download all attachments as: .zip

Change History (11)

@emirpprime
3 years ago

#1 @hellofromTonya
3 years ago

  • Keywords has-patch needs-unit-tests added
  • Version changed from trunk to 2.2

#2 @desrosj
2 years ago

#53699 was marked as a duplicate.

#3 @kebbet
6 months ago

#58003 was marked as a duplicate.

#4 @kebbet
6 months ago

#58002 was marked as a duplicate.

#5 @rajinsharwar
7 weeks ago

  • Keywords needs-dev-note added
  • Milestone changed from Awaiting Review to 6.4
  • Version 2.2 deleted

Putting this for 6.4

This ticket was mentioned in PR #5037 on WordPress/wordpress-develop by @rajinsharwar.


6 weeks ago
#6

  • Keywords has-unit-tests added; needs-unit-tests removed

Unit tests for resetting email with an apostrophe

Trac ticket: https://core.trac.wordpress.org/ticket/52529

#7 @rajinsharwar
6 weeks ago

  • Keywords needs-unit-tests added; has-unit-tests removed

Reverting the units tests, need to check further for failures.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


3 days ago

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


3 days ago

#10 @rajinsharwar
2 days ago

  • Keywords needs-dev-note removed

As per the discussions on the Bugs Scrub of today (09/28/2023), the current patch was proposed to be tested if it was working correctly, and if it was working fine with the current upstream.

After testing the current patch, I found that the patch is working as expected, and I am able to reset the password for a user who has an apostrophe in his/her email.

@oglekler @mukesh27 Do you think we need unit tests for this scenario?

Note: See TracTickets for help on using tickets.