Ticket 34753, Document how to properly escape `to` in email messages

Django Internals
1

Hello,

I was looking at

because I had the same problem, easily reproduced:

from django.core.mail import send_mail

send_mail(
    "Hello",
    "message",
    "from@example.com",
    ["last_name, first_name <info@test.com>"],
    fail_silently=False,
)

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/testmail/.venv/lib/python3.12/site-packages/django/core/mail/__init__.py", line 92, in send_mail
    return mail.send()
           ^^^^^^^^^^^
  File "/home/testmail/.venv/lib/python3.12/site-packages/django/core/mail/message.py", line 307, in send
    return self.get_connection(fail_silently).send_messages([self])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/testmail/.venv/lib/python3.12/site-packages/django/core/mail/backends/smtp.py", line 136, in send_messages
    sent = self._send(message)
           ^^^^^^^^^^^^^^^^^^^
  File "/home/testmail/.venv/lib/python3.12/site-packages/django/core/mail/backends/smtp.py", line 151, in _send
    sanitize_address(addr, encoding) for addr in email_message.recipients()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/testmail/.venv/lib/python3.12/site-packages/django/core/mail/message.py", line 93, in sanitize_address
    raise ValueError(
ValueError: Invalid address; only last_name could be parsed from "last_name, first_name <info@test.com>"

This is especially problematic when the to-string is constructed like

to_addr = f"{name} <{email}>"

where name and email are from model fields – where at least name is normally not pre-sanitized for use in an email address.

The original reporter suggested to (only) add a note the documentation, so can we reopen?

1 Like
2

I’m +1 on reopening the ticket and documenting how to do this safely.

Did you see this approach suggested somewhere in the Django docs or code? If so, we need to fix it. (If you saw it somewhere else, please try to get them to fix it.)

Constructing an email address through string concatenation is a security hole. It allows the email equivalent of SQL or HTML injection. (Unfortunately, it’s also pretty common—even some well-known ESPs’ APIs have been vulnerable.)

The safe way to format a display name and email into an address is by using the Address object from Python’s modern email API:

from email.headerregistry import Address

to_addr = str(Address(display_name=name, addr_spec=email))

# E.g.:
str(Address(
    display_name="last name, first_name", 
    addr_spec="info@test.com"))
# '"last name, first_name" <info@test.com>'

I’d be in favor of adding an example to Django’s email docs showing how to safely construct an email address. Maybe as part of the Preventing header injection section (which currently isn’t all that informative, and seems maybe a holdover from the days before Django prevented email header injection). While it’s true that Django can’t document every caveat about sending email, this seems like a common footgun for developers using Django’s core mail APIs.

3

Maybe the API should raise a deprecation warning and later raising an error if you do this and point the user to using Address instead?

4

Well, no, the stupidity is all mine. :blush:

But then Sending email | Django documentation | Django says:

from_email: The sender’s address. Both fred@example.com and "Fred" <fred@example.com> forms are legal.

which seems to suggest that this format is at least not discouraged.

More generally, when examples like

to_addresses = ["first@example.com", "other@example.com"]

appear throughout the docs, it’s hard to infer that

to_addresses = ["First <first@example.com>", "Other <other@example.com>"]

would be considered questionable.

5

This ticket now got a bit more weird because the ADMINS/MANAGERS settings in Django 6.0 says the tuple format is deprecated, forcing you to use the single string.. which seems unsafe according to this discussion? django/django/core/mail/__init__.py at 0ca3a0661173b02e2cbb0183d8543e790e7e4a55 · django/django · GitHub

6

It’s fine to provide literal email address strings, with or without display names. The problem is building an address string from user-supplied parts:

# Correct, and safe (pay close attention to the quotes):
to_addresses = ["first@example.com", "other@example.com"]
to_addresses = ["First <first@example.com>", "Other <other@example.com>"]
to_addresses = ['"Last, First" <first@example.com>', '"Other (work)" <other@example.com>']

# Wrong, but not unsafe:
to_addresses = ["Last, First <first@example.com>"]  # ValueError
to_addresses = ["Other (work) <other@example.com>"]  # "(work)" gets lost

# NOT SAFE!
to_addresses = [f"{name} <{email}>"]

# ALSO NOT SAFE!
to_addresses = [f'"{name}" <{email}>']

Email address header fields have their own, specialized, complex syntax (they even support comments!). Like SQL or HTML or anything with a syntax, if you try to construct one by assembling strings—without being aware of that syntax—you’re leaving yourself open to an injection vulnerability.

It’s a form of email header injection: if the {name} is user-supplied content, an attacker can use it to change where the message is sent, by adding an additional email or even hiding the {email} part in a comment so it gets ignored.[1]

Because the email address syntax is meant to be something humans can read and write, it’s easy for developers to assume they can build it from string parts.[2] But that would be a mistake—just as it is with SQL or HTML.

Again, properly formatted string literals are safe, and improperly formatted string literals won’t work (but aren’t unsafe). The updated settings docs deliberately include an example with a couple of properly formatted email address strings.

If we end up with a note in the email docs about how to safely compose an email address string (and I think we should), it would be great to link to that from the ADMINS/MANAGERS settings docs.

(Incidentally, the change from tuples to strings in Django 6.0 was because the “name” part of the ADMINS/MANAGERS tuples had never been used, in any released version of Django[3]. Name was just ignored. And the “email” part was assumed to be a fully formatted address—possibly including a display name—without additional validation.)

  1. name = 'spy@evil.com, ' or name = 'spy" <spy@evil.com>, (' ↩︎

  2. A shockingly large number of commercial email service providers have made that incorrect assumption in their APIs. Despite being (presumably) experts in email. ↩︎

  3. I’m guessing an unreleased, early Django used Python’s email.utils.formataddr(), which takes (name, email) tuples, on ADMINS and MANAGERS. I’m not sure how or why that code was lost, but the settings remained unchanged until Django 6.0. ↩︎

2 Likes
7

I reopened & accepted the ticket based on this thread and Mike’s advice that “Preventing header injection” needs a face-lift.

1 Like
8

Based on what was in the docs that was a bug though…? I believe that at least the ADMINS/MANAGERS could be backward compatible, since the list of tuples was enforced there and documented as such

9

That was (briefly) discussed here in Simplifying ADMINS and MANAGERS settings?. At the time, there were bugs in Python’s Address object that would have prevented using it for this purpose without creating backwards compatibility issues. (And those bugs are still there in some of the Python versions supported by Django 5.2, which was the target at the time of the discussion and PR submission.)

10

Can we make the APIs accept Address objects as well as str? That would act as an attractor towards the safe method, and provide a clear documentation point for when to use Address vs str.

2 Likes
11

I’m pretty sure most of the django.core.mail APIs already do accept Address objects. For recipient fields since at least 5.2 (probably earlier), and for from_email starting in 6.0. It’s just not tested or documented.[1]

There’s code underlying ADMINS/MANAGERS that specifically checks for str to complain about misconfiguration, so that would need to be updated.

  1. EmailMessage._set_list_header_if_not_empty() coerces each to/cc/bcc/reply_to list item to a str, which makes Address objects work with Python’s legacy or modern email APIs. The from_email isn’t converted to a string, so using an Address there only works in 6.0+ with the modern email API. ↩︎

1 Like