Skip to content
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

Omit \r\r\n #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Omit \r\r\n #76

wants to merge 1 commit into from

Conversation

ionum
Copy link

@ionum ionum commented Jun 20, 2021

Check if endline is already \r\n

Copy link
Owner

@bruceg bruceg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simple enough, but I have a couple of questions:

  1. Would it make sense for it to be a while instead of an if to effect stripping all trailing \r?
  2. It would be good to test this behavior. Have you taken a look at doing that?

@ionum
Copy link
Author

ionum commented Mar 22, 2022

  1. I don't think so. Mail-delivering-programms use \r\n or \n as line terminators. * getline splits on \n so only one \r remains at the end of the line if the Mail-delivering-programm uses \r\n
  2. I wrote that patch because my mails got crippled. I'm testing my patch since Jun 2021 :-). No problems so far.

*RFC 2821: SMTP commands and, unless altered by a service extension, message
data, are transmitted in "lines". Lines consist of zero or more data
characters terminated by the sequence ASCII character "CR" (hex value
0D) followed immediately by ASCII character "LF" (hex value 0A).
This termination sequence is denoted as in this document.
Conforming implementations MUST NOT recognize or generate any other
character or character sequence as a line terminator. Limits MAY be
imposed on line lengths by servers (see section 4.5.3).

In addition, the appearance of "bare" "CR" or "LF" characters in text
(i.e., either without the other) has a long history of causing
problems in mail implementations and applications that use the mail
system as a tool. SMTP client implementations MUST NOT transmit
these characters except when they are intended as line terminators
and then MUST, as indicated above, transmit them only as a
sequence.

Check if endline is already \r\n
@maxpunktezahl
Copy link

maxpunktezahl commented Nov 20, 2023

Just an additional hint. I anybody wan't to use the (great!) nullmailer with s/mime encoded mails like me, ionums patch is absolutely essential. All attempts to sign (end encrypt) mails with 'openssl smime' will fail without the patch! See https://stackoverflow.com/questions/66228622/how-to-use-openssl-to-sign-an-email-message-and-curl-to-send-it
The pached smtp lib worked instantly with s/mime.

@bmork
Copy link
Contributor

bmork commented Oct 2, 2024

Please merge this. It fixes errors like this one:

2024-10-02T12:39:11.650699+02:00 miraculix nullmailer-send[1506356]: smtp: Failed: 421 4.5.0 Bare carriage return (CR) not allowed

which showed up regularily after my sendmail based smart relay got a fix for CVE-2023-51765. Typically caused by a cron job printing a message ending in \r.

I believe the problem can be reproduced by simply doing:

echo -e "To: [email protected]\n\nFoo\r"|nullmailer-inject

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants