Nodebb: Users able to signup with invalid email address (containing comma)

[fais3000]

In ref to https://community.nodebb.org/topic/11165/user-able-to-register-with-invalid-email-address

Users are able to create an account via local signup providing an invalid email address. e.g. "abc@gm,com". Once the account is created, it can't be removed from ACP or User Profile unless the email is changed.

[barisusakli]

I was able to ban and delete a user account with the email abc@def,com Do you have any custom plugins that might be causing this? Please also provide your git hash.

[fais3000]

@barisusakli Git hash 956df9d7d7677e313b7f11c848be5e7b772327d7

Active plugins:
- nodebb-theme-persona
- nodebb-plugin-composer-default
- nodebb-plugin-markdown
- nodebb-plugin-mentions
- nodebb-widget-essentials
- nodebb-rewards-essentials
- nodebb-plugin-dbsearch
- nodebb-plugin-emailer-mailgun
- nodebb-plugin-sso-facebook
- nodebb-plugin-sso-google
- nodebb-plugin-sso-twitter
- nodebb-plugin-custom-homepage
- nodebb-plugin-category-sort-by-votes
- nodebb-plugin-custom-pages
- nodebb-plugin-tws
- nodebb-plugin-google-analytics
- nodebb-plugin-imgur
- nodebb-plugin-imagemagick
- nodebb-plugin-emoji-extended
- nodebb-plugin-write-api
- nodebb-plugin-shoutbox
- nodebb-plugin-emoji-one
- nodebb-plugin-giphy
- nodebb-plugin-ns-custom-fields
- nodebb-plugin-audio-embed
- nodebb-plugin-soundpack-default
- nodebb-plugin-soundcloud
- nodebb-plugin-audio-control
- nodebb-plugin-ns-embed
- nodebb-plugin-embed-comboh

[barisusakli]

Are you on redis or mongodb?

[fais3000]

@barisusakli MongoDB

[barisusakli]

I tested on mongodb as well, but I had a lot less plugins activated, mainly the default ones. Can you disable the plugins and see if the issue is fixed.

[fais3000]

@barisusakli Ok, I'll check and report back.

What are your thoughts on letting the users signup with an email containing ","? Isn't it invalid and should not be allowed to pass through?

[julianlam]

We only test that an email contains @, any more is needlessly exhaustive and will lead to false positives (Google "email regex").

The best way to verify an email is to send an email to it, hence the email confirmation step.

[pitaj]

Maybe delete users who don't confirm their email within a certain amount of time?

(This may already be a feature)

[pauljherring]

What are your thoughts on letting the users signup with an email containing ","? Isn't it invalid and should not be allowed to pass through?

While unusual, and sometimes problematic, they are allowed (in the local-part, at least):

• https://tools.ietf.org/html/rfc5322#section-3.4.1
• https://tools.ietf.org/html/rfc5321#section-4.1.2

[fais3000]

@julianlam @barisusakli Alright, theoretically makes sense for validation, though causes a bit of a problem with shoutbox as these fake accounts try to post/spam (even without email confirmation), which I understand is more of a shoutbox plugin issue.

On the issue of deletion of those accounts, I tracked, the exception is being thrown by these:

[5259] - mailer.mailgun] ('to' parameter is not a valid address. please check documentation)
[5259] - lugins] filter:email.send, 'to' parameter is not a valid address. please check documentation

Perhaps an email for "Ban" is scheduled to go on ban action. Do you guys have any suggestions what is the best measure in cases like these?

[Radiergummi]

@julianlam Wouldn't it make sense to extend the email verification to checking whether the address contains a valid hostname after the last @? That should be easier since the specs for domains are way less complicated, one could even use the DNS module and try to resolve the domain. That would also prevent having to send an email to a definitely invalid address.

[pauljherring]

one could even use the DNS module and try to resolve the domain.

The canonical way of doing that check is to see if there's an MX record for the domain (i.e. does it accept email.) In php it's checkdnsrr($domainname, "MX");, node.js seems to be dns.resolveMx(hostname, callback);

However

• DNS lookups can be slow at times
• It can result in false negatives (i.e. there is an SMTP server on the IP address in the A record, but the MX record hasn't been configured.)

All depends on how certain you want to be of the address, and - really - the only way of being certain about an email address being valid is to try and send mail to it with a UUID that the user sends back somehow.

[Radiergummi]

Going the last mile and checking for an MX record would be the definite answer, even though skipping the DNS verification and just validating the given FQDN seems more attractive to me (see here for a regex that satisfies the specs).
I'm not too much into NodeBB's core to be qualified to tell whether a DNS query or regex validation costs more than rendering and dispatching an email, so maybe my whole point is moot, in which case, just ignore me 😄.

[pauljherring]

More pedantry.. that regex fails for (from a quick test) 36 FQDN's (That regex requires a period within the name - there are 36 TLDs that are FQDNs.)

:smiling_imp:

[Radiergummi]

I'm sorry for spamming this issue but my curiosity won - I never knew there are any routable TLDs that are FQDNs?! Do you have a source for that?

[pauljherring]

Do you have a source for that?

Only what I just did to get the list:

Get a list of TLD's
wget http://www.iana.org/domains/root/db

Grab them out of the page
grep "domain tld" db | sed "s/^.*>\.\([^<]*\)<.*/\1/g" | grep -v class >db2

Find out which of them return a non-zero number of addresses
(while read tld; do echo "$tld"; dig $tld. | grep "ANSWER: [^0]"; done <db2) | tee db3

Count them.

grep ";;" db3 -B1  | egrep -v "\-\-|flags" | wc -l
36

Full list for the really interested:

for tld in $(grep ";;" db3 -B1  | egrep -v "\-\-|flags"); do dig $tld |  grep "^[^;]" | grep "[0-9]*\."; done
ai.                     13078   IN      A       209.59.119.34
android.                2384    IN      A       127.0.53.53
anquan.                 2384    IN      A       127.0.53.53
arab.                   2385    IN      A       127.0.53.53
cal.                    2419    IN      A       127.0.53.53
chrome.                 2425    IN      A       127.0.53.53
cm.                     85228   IN      A       195.24.205.60
dclk.                   2488    IN      A       127.0.53.53
dev.                    2490    IN      A       127.0.53.53
dk.                     85292   IN      A       193.163.102.58
drive.                  2494    IN      A       127.0.53.53
etisalat.               2499    IN      A       127.0.53.53
gg.                     85311   IN      A       87.117.196.80
gle.                    2512    IN      A       127.0.53.53
grocery.                2516    IN      A       127.0.53.53
guge.                   2516    IN      A       127.0.53.53
hangout.                2518    IN      A       127.0.53.53
je.                     85330   IN      A       87.117.196.80
map.                    2545    IN      A       127.0.53.53
merckmsd.               2547    IN      A       127.0.53.53
nexus.                  2557    IN      A       127.0.53.53
nowruz.                 2558    IN      A       127.0.53.53
pars.                   2563    IN      A       127.0.53.53
phd.                    2565    IN      A       127.0.53.53
play.                   2567    IN      A       127.0.53.53
pn.                     42168   IN      A       80.68.93.100
politie.                767     IN      A       127.0.53.53
search.                 2581    IN      A       127.0.53.53
shia.                   2584    IN      A       127.0.53.53
shouji.                 2584    IN      A       127.0.53.53
tci.                    2595    IN      A       127.0.53.53
tk.                     288     IN      A       217.119.57.22
uz.                     13404   IN      A       91.212.89.8
ws.                     20612   IN      A       64.70.19.33
xihuan.                 2613    IN      A       127.0.53.53
yun.                    2623    IN      A       127.0.53.53

Not all resolve to a useful address as can be seen, but still...

[pitaj]

We do use validator.isEmail on the backend to validate the email.

[nodebb-misty]

Issue closed due to inactivity.

---

This is an automated message. If you feel this action was in error, please comment on this issue so it can be looked at again