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

Improve email validation #622

Closed
Lesterpig opened this Issue Apr 22, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@Lesterpig

Lesterpig commented Apr 22, 2018

The current email validation function is very simple, but does not accept every valid email address according to RFCs (https://en.wikipedia.org/wiki/Email_address#Syntax). I can submit a PR if you are OK to improve the check_email_format function and linked test.

return bool(re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email))

Example: ~@provider.com shall be accepted.

This issue may be linked to #603

@ColdHeat

This comment has been minimized.

Member

ColdHeat commented Apr 22, 2018

Well I don't know how you are going to improve upon the email validation because I believe the best practice is to confirm emails.

https://stackoverflow.com/questions/201323/how-to-validate-an-email-address-using-a-regular-expression
http://www.regular-expressions.info/email.html

PRs are always welcome but I'm not sure you can improve upon the email validation in a simple way. I'm not particularly interested in debugging a massive email regex.

@Lesterpig

This comment has been minimized.

Lesterpig commented Apr 23, 2018

Thanks for your reply!

Additionally, it should be OK to accept the following international (or special) email addresses:

用户@例子.广告
user+mailbox/department=shipping@example.com
🐷@provider.com

The simplest way to filter user's errors would be to check only for the @ and . chars while disallowing whitespaces. The real verification would be in the verify_email function 😄

[^@\s]+@[^@\s]+\.[^@\s]+
@ColdHeat

This comment has been minimized.

Member

ColdHeat commented Apr 23, 2018

This honestly sounds like a risky idea but from my research it seems like the future™.

https://www.theregister.co.uk/2014/08/07/gmail_goes_international_with_rfc_6530_support/
https://mcpmag.com/articles/2018/01/03/exchange-online-email-internationalization.aspx

However I might add that Chrome does not accept your example email addresses:
http://jsfiddle.net/dbd8paxa/

RFC6530, the RFC that is relevant here, is a proposed standard so the question I suppose is whether or not this is worth it considering that Chrome does not accept these test emails as valid.

@superboum I see you gave a thumbs up, does this issue affect you? Do you work around it for other services?

@superboum

This comment has been minimized.

superboum commented Apr 23, 2018

As I did a CTF in team with @Lesterpig, the issue (~@provider.com) affected our whole team.
Even if some examples feel very unlikely, there are often cases where it makes sense to use these strange email addresses. Especially when your targets are CTF players 😄

Couldn't a really simple regex be enough like the one proposed by Lesterpig? In any case, if you really need to validate an email address, the only way is to send an email - and CTFd already has the feature.

@ColdHeat

This comment has been minimized.

Member

ColdHeat commented Apr 23, 2018

It could work but I am unsure of the underlying effects it would have. This would reduce the amount of validation that CTFd does which to me sounds like a risky idea.

If there are no underlying issues that arise from displaying invalid emails, security or otherwise I would be okay with accepting a PR for this. Feel free to initiate the PR if you'd like.

Lesterpig added a commit to Lesterpig/CTFd that referenced this issue Oct 17, 2018

Relax email regex validation rule
The real email validation is done via email confirmation.
This commit proposes a relaxed regex to allow for Unicode and more
esoteric, yet valid, email addresses.

Web frontend is expected to protect users against injections contained
in emails.

Fix CTFd#622

Lesterpig added a commit to Lesterpig/CTFd that referenced this issue Oct 17, 2018

Relax email regex validation rule
The real email validation is done via email confirmation.
This commit proposes a relaxed regex to allow for Unicode and more
esoteric, yet valid, email addresses.

Web frontend is expected to protect users against injections contained
in emails.

Fix CTFd#622

Lesterpig added a commit to Lesterpig/CTFd that referenced this issue Oct 18, 2018

Relax email regex validation rule
The real email validation is done via email confirmation.
This commit proposes a relaxed regex to allow for Unicode and more
esoteric, yet valid, email addresses.

Web frontend is expected to protect users against injections contained
in emails.

Fix CTFd#622

Lesterpig added a commit to Lesterpig/CTFd that referenced this issue Oct 18, 2018

Relax email regex validation rule
The real email validation is done via email confirmation.
This commit proposes a relaxed regex to allow for Unicode and more
esoteric, yet valid, email addresses.

Web frontend is expected to protect users against injections contained
in emails.

Fix CTFd#622
@Lesterpig

This comment has been minimized.

Lesterpig commented Oct 29, 2018

Fixed in #693, thanks!

@Lesterpig Lesterpig closed this Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment