Make WordPress Core

Opened 2 years ago

Last modified 17 months ago

#44683 reviewing enhancement

Export and Erase personal data - emails sent to wrong address if username is an email address which is different from the actual email address

Reported by: subrataemfluence Owned by: garrett-eclipse
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords:
Focuses: Cc:


This issue might had been already discussed in another ticket but I was unable to find it.

When a user has set his username to an email address (may not be a valid one), then WordPress will be forced to send the verification request to wrong one.

Here is an example:

Username: fake@bbbb.com

For the above, the following code block won't execute:

if ( ! is_email( $username_or_email_address ) ) {

but this line will be executed:

$email_address = $username_or_email_address;

Which means email is now supposed to be sent at fake@bbbb.com.

I would suggest even a username looks to have a valid email address pattern like above, rather than directly assign it against $email_address variable, we might take an extra step to find the real email address attached to this account before sending the request.

Here is a suggestion:

if ( ! is_email( $username_or_email_address ) ) {
} else {
   $user = get_user_by( 'login', $username_or_email_address );
   if ( $user instanceof WP_User ) {
      $email_address = $user->user_email;
   } else {
      $email_address = $username_or_email_address;

Please let me know if this makes any sense!

Change History (8)

#1 follow-up: @knutsp
2 years ago

This may get wrong when username is, or looks like, an email address?

Usernames that looks like email address should be prohibited, but that's another ticket.

#2 in reply to: ↑ 1 @subrataemfluence
2 years ago

Replying to knutsp:

This may get wrong when username is, or looks like, an email address?

Exactly! Problem starts when username is an email address or look alike.

Usernames that looks like email address should be prohibited, but that's another ticket.


#3 @subrataemfluence
2 years ago

  • Component changed from Gallery to Privacy

#4 @pento
18 months ago

  • Version trunk deleted

#5 @garrett-eclipse
18 months ago

  • Version set to 4.9.6

#6 @desrosj
17 months ago

  • Keywords GDPR removed
  • Owner set to desrosj
  • Status changed from new to assigned

Thanks for reporting this, @subrataemfluence. I am assigning to myself to investigate further. Also, for future reference, the gdpr keyword is no longer used. Using the Privacy componennt or privacy focus is sufficient.

#7 @desrosj
17 months ago

  • Keywords needs-unit-tests added

If you are able to, @subrataemfluence, writing a unit test that proves this is not working correctly would be helpful.

#8 @garrett-eclipse
17 months ago

  • Keywords 2nd-opinion needs-unit-tests removed
  • Owner changed from desrosj to garrett-eclipse
  • Status changed from assigned to reviewing

Thanks @subrataemfluence

This is related to #44347 and I feel needs to be addressed there. I was able to reproduce the issue though but I feel it makes more sense to introduce rigid controls over emails as usernames than it does to attempt to anticipate it within the tools.

It's going to get confusing and potentially leak data to the wrong user when usernames are emails but are of other users.
One thing I found potentially concerning was with registration open I was able to signup a user using my admin email as their username.

I agree with @knutsp that usernames should prohibit the user of email addresses. or at worst limit the username to be an exact match of their email. I'm posting a note to the other ticket and leaving this open for the time being in case those changes require anything to be done to the Privacy tools.

Note: See TracTickets for help on using tickets.