Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users with emails that contain apostrophes cannot log in. #120

Open
brettshumaker opened this issue May 28, 2021 · 2 comments · May be fixed by #144
Open

Users with emails that contain apostrophes cannot log in. #120

brettshumaker opened this issue May 28, 2021 · 2 comments · May be fixed by #144

Comments

@brettshumaker
Copy link

A user with an apostrophe in their email address can't properly log in. wp_slash() is being used here on the email address: https://github.com/onelogin/wordpress-saml/blob/master/onelogin-saml-sso/php/functions.php#L308-L309

This adds a \ before the apostrophe, so when it uses email_exists() with the slashed email address, even if one exists, it can't find it and creates a new user. Because \ is not a valid character in an email address, it's stripped out when creating the user. So when the user tries to log in a 2nd time, the plugin is still using the slashed email address to see if the email exists and still doesn't find one and tries to create another user. This time, however, since the username already exists, WordPress catches it and shows the "Sorry, that username already exists!" message.

It looks like those lines have been in this plugin for around 7 years, but I don't think they're necessary because the user_login and email address should have already been run through the proper sanitization functions, and wp_slash() itself shouldn't really be used for sanitization.

@pitbulk
Copy link
Contributor

pitbulk commented Jun 4, 2021

At the wp-login.php I see that wp_unslash is used instead:

https://github.com/WordPress/WordPress/blob/master/wp-login.php#L959

So I guess that should be the method used, wp_unslash instead of wp_slash
and in order to sanitize, sanitize_user for the username

Do you want to provide a PR?

@kempersan
Copy link

Wanted to follow up on this issue. I'm having the same problem with users that have an apostrophe in their name for version 3.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants