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

E-mail address validation regex for ADMIN_EMAIL in initialData is wrong #2841

Closed
henryk opened this issue Apr 12, 2016 · 1 comment · Fixed by #2890
Closed

E-mail address validation regex for ADMIN_EMAIL in initialData is wrong #2841

henryk opened this issue Apr 12, 2016 · 1 comment · Fixed by #2890
Assignees
Milestone

Comments

@henryk
Copy link
Contributor

henryk commented Apr 12, 2016

Your Rocket.Chat version: 0.25.0.

Steps to reproduce

  1. Try to create an initial admin with any of a number of uncommon, but valid e-mail address syntaxes. E.g.
~/Rocket.Chat$ ADMIN_PASS=Password ADMIN_EMAIL=me+rocket@mydomain.example node main.js

(Note: I'm using mydomain.example to prevent spam-harvestable e-mail addresses here. In reality I've been using an actual domain name.)

Actual results

Inserting admin user:
Name: Administrator
E-mail provided is invalid; Ignoring environment variables ADMIN_EMAIL
Username: admin
Password: Password

Expected results

Should have used the e-mail address I have provided.

Further information

The core problem is in

re = /^([\w-]+(?:\.[\w-]+)*)@((?:[\w-]+\.)*\w[\w-]{0,66})\.([a-z]{2,6}(?:\.[a-z]{2})?)$/i

That validation regex is fundamentally broken six ways from Sunday:

  • It doesn't allow any characters except for a-z0-9, "-" and "." in the local part.
  • It enforces strange restrictions on the domain part (must have a character, followed by a dot, followed by 2 to 6 characters). Note that TLDs can be much longer that 6 characters, see https://data.iana.org/TLD/tlds-alpha-by-domain.txt

The combination of both issues prevents many e-mail addresses that would work perfectly fine from being used, for no apparent reason.

For more information on the complexity of allowed characters in e-mail addresses see https://en.wikipedia.org/wiki/Email_address#Examples

Recommended solution

I posit that it's absolutely useless to try to 'verify' an e-mail address entered by the user. Their address is what they say it is. Even if it is #@to (Yes, that's a syntactically valid address that's deliverable in principle). The only checks you may safely make are:

  • There must be an "@"
  • There must be something in front of and behind the "@"

Therefore the abovementioned regex should be replaced with:

/^.+@[^@]+$/
@engelgabriel
Copy link
Member

Can you submit a PR for that?

@engelgabriel engelgabriel added this to the 0.27.0 milestone Apr 12, 2016
@engelgabriel engelgabriel self-assigned this Apr 12, 2016
henryk added a commit to henryk/Rocket.Chat that referenced this issue Apr 14, 2016
 * Email addresses start with something other than "@", have arbitrary characters,
   at least one "@", at least one character after that.
engelgabriel pushed a commit that referenced this issue Apr 14, 2016
* Email addresses start with something other than "@", have arbitrary characters,
   at least one "@", at least one character after that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants