Opened 7 years ago

Closed 3 years ago

#12027 closed Bug (fixed)

EmailField validation is incorrect, trailing dots fail

Reported by: Klas H Owned by: shaz
Component: Core (Other) Version: master
Severity: Normal Keywords: email, regular expression, validation
Cc: joshcartme@…, timograham@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the email field related denial-of-service attack was patched the new code suddenly allowed email addresses to end with a '.' character. For example, according to the new regular expression 'test@test.com.' is a valid email address (please note the trailing period character).

I attach a patch that fixes this.

Attachments (5)

email_re_patch.diff (1.0 KB) - added by anonymous 7 years ago.
Patch of regular expression for email address validation
12027_patch_test.diff (820 bytes) - added by shaz 4 years ago.
Test patch for ticket
12027_patch_test.diff​ (1.1 KB) - added by shaz 4 years ago.
12027.diff (1.5 KB) - added by Tim Graham 3 years ago.
12027_regex.diff (1.2 KB) - added by Tim Graham 3 years ago.

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by anonymous

Attachment: email_re_patch.diff added

Patch of regular expression for email address validation

comment:1 Changed 7 years ago by Klas H

Cc: Jacob removed
Resolution: invalid
Status: newclosed

Ignore this ticket. Apparently the domain part of an URL can end with a '.' character, so I guess it's the same for mail addresses then.

comment:2 Changed 5 years ago by Josh C

Cc: joshcartme@… added
Easy pickings: unset
Resolution: invalid
Severity: Normal
Status: closedreopened
Summary: EmailField validation is incorrectEmailField validation is incorrect, trailing dots fail
Type: Uncategorized
UI/UX: unset

I'm not convinced this should be closed. Although DNS should correctly resolve a domain with a trailing dot according to the specification (documented at the beginning of page 7 here http://www.ietf.org/rfc/rfc1034.txt) sending emails to addresses that end with a dot fails at least in my case. For example I tried sending an email to and from a google apps address of mine using thunderbird. I was sent back a Delivery Status Notification (Failure).

When trying to use EmailMultiAlternatives with a email address of this form, me@…. as an example, msg.send() breaks with the following error:
SMTPRecipientsRefused: {'me@….': (501, '<tedlittle5@….>: domain missing or malformed')}

Here is a quote from RFC 5321 section 2.3.5 (http://tools.ietf.org/html/rfc5321#section-2.3.5):

"A domain name (or often just a "domain") consists of one or more components, separated by dots if more than one appears."

I think the important word is separated, it does not say terminated therefore an email address should not be terminated by a dot.

More discussion on that here: http://tech.groups.yahoo.com/group/postfix-users/message/244166.

At the very least I think this needs to be revisited and properly discussed. Emails with dots at the end do not send so why should they be validated by Django?

comment:3 Changed 5 years ago by Julien Phalip

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

#16310 was a duplicate. Note that this behaviour was introduced in changeset [11605], which itself was to fix a security issue where the email and url validation regular expressions could be exploited in public form submissions to cause a DOS. Therefore this should be treated very cautiously.

I haven't done enough research to confirm whether this is a genuine bug or not. However, this behaviour (i.e. trailing dots) doesn't seem to be tested at all. So I'm accepting this ticket on the basis that at the very least it needs some tests.

comment:4 Changed 5 years ago by Julien Phalip

Needs tests: set

comment:5 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

Changed 4 years ago by shaz

Attachment: 12027_patch_test.diff added

Test patch for ticket

comment:6 Changed 4 years ago by shaz

Triage Stage: AcceptedReady for checkin

Changed 4 years ago by shaz

Attachment: 12027_patch_test.diff​ added

comment:7 Changed 4 years ago by shaz

Owner: changed from nobody to shaz
Status: newassigned

comment:8 Changed 4 years ago by Tim Graham

Cc: timograham@… added
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Please don't mark your own patch as RFC. It looks like this needs to be discussed as to whether we want to continue to allow email addresses that end with periods or not. If so, the test in the latest patch is a bit awkward (using both unittest.expectedFailure and assertFailsValidation).

comment:9 Changed 3 years ago by Ole Laursen

We just had an SMTP exception from Exim because a user managed to enter his email address with a trailing dot. Probably he copied it from somewhere with a period at the end. This echoes what Josh C said, but I just want to confirm that Django's behaviour in this case caused us a real problem, so yeah, this is a genuine bug.

Changed 3 years ago by Tim Graham

Attachment: 12027.diff added

comment:10 Changed 3 years ago by Tim Graham

Needs tests: unset
Patch needs improvement: unset

Here's a simple patch that avoids messing with the regex. Perhaps it's good enough.

Changed 3 years ago by Tim Graham

Attachment: 12027_regex.diff added

comment:11 Changed 3 years ago by Tim Graham

And now a version that modifies the regex. Thanks @claudep

comment:12 Changed 3 years ago by Claude Paroz

Triage Stage: AcceptedReady for checkin

comment:13 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 4cfbde71a33973cb8c8da406b2523dfc742358e7:

Fixed #12027 -- Fixed EmailValidator to reject a trailing dot.

Thanks Klas H for the report and claudep for the patch.

Note: See TracTickets for help on using tickets.
Back to Top