Allow email sending with a newline in the subject

[Álex Córcoles]

Hi,

django.core.mail.send_mail raises an exception if the subject contains a newline.

We have hit this problem because we have a piece of code that parses an email using Python's libraries, extracts the subject and then re-sends an email with the same subject.

If this email has a multi-line subject (this seems to be common, I believe some bit in mail delivery code wordwraps headers), this code then fails unexpectedly.

This is extremely hard to detect before it happens, so we would humbly suggest that send_mail don't raise an error if passed a subject containing a newline, but rather sends the email doing the necessary security handling.

Kind regards,

Álex

[Tim Graham]

What does "the necessary security hardening" mean?

[Carlton Gibson]

Hmmm. I seem to recall that allowing newlines leads to dangers of email injection attacks… (I’d need to look that up).

​RFC 2822 section 2.2.3 says:

The process of moving from this folded multiple-line representation
of a header field to its single line representation is called
"unfolding". Unfolding is accomplished by simply removing any CRLF
that is immediately followed by WSP. Each header field should be
treated in its unfolded form for further syntactic and semantic
evaluation.

i.e. I think you’re meant to remove the newlines before resending the email.

I’m going to close on that basis.

[Álex Córcoles]

Yeah, I traced the code and I was that avoiding sending an email with newlines had to do with security.

However, I think that raising an undocumented exception is not the right answer.

One option would be to document this problem clearly in send_mail's docs. I don't think that's going to save many people from possible runtime problems, but at least the behavior is documented.

I would propose that send_mail removes itself the newlines instead of pushing that responsibility to each of its callers. I think it's consistent with all the ORM methods being able to avoid SQL injection instead of raising an undocumented exception if they receive non-escaped values.

[Carlton Gibson]

Putting newlines in email headers is known No-no, or it should be. (That the transport layer might wrap headers as per the spec is something else entirely.)
Adding behaviour to handle bad input isn't something I can see as sensible.

What's the actual error you get? Yes, a PR with a better error message would always be worth a look. (But pending such a PR this isn't an active issue.)

[Álex Córcoles]

Oh, the exception itself is quite clear:

​https://github.com/django/django/blob/master/django/core/mail/message.py#L60

; it doesn't provide a rationale, but I don't think an exception should do that- so I wouldn't really touch that, it's perfect as it stands.

I'd suggest documenting this behavior clearly in the docs (the docs hint that it can throw SMTPExceptions, but this is a ValueError). I'd rather join newlines, but if you guys decide this is good as it stands, I have nothing more to say.

Thanks for your time,

Álex

[Tim Graham]

I guess you didn't see the ​Preventing header injection section.

If you want to make a proposal about changing the behavior, the DevelopersMailingList would be the place to do it. A more concrete proposal than "the necessary security hardening" is needed. If there's a consensus, then we'd reopen this ticket.

I didn't see anything in the original ticket (#1135) about the implementation (vs. trying to do some sanitization). If other frameworks have a different behavior, it may make sense to follow their pattern. I agree it can be a bit cumbersome to catch BadHeaderError or write your own sanitization.

[Álex Córcoles]

I stand corrected, indeed I had not seen that section which addresses my concerns for documentation correctly.

As you say:

I agree it can be a bit cumbersome to catch BadHeaderError or write your own sanitization.

, I see you understand correctly the issue I'm having , so while I disagree, I think pursuing this further is not using anyone's time effectively.

Thanks for your time,

Álex