Bug-Debian: http://bugs.debian.org/584162 Reported-By: Christoph Biedl Forwarded: not-needed Reviewed-By: Anibal Monsalve Salazar Last-Update: 2014-08-15 From: "Daniel Richard G." Subject: Re: ssmtp: Partial loss of message body, sending message to wrong recipicients Date: Thu, 19 Jun 2014 14:44:30 -0400 Attached is a patch against the original 2.64 source that should address this bug, and hopefully not break anything. An overview of my changes: * Added code to standarise() to drop the trailing '\r' if the line originally ended with "\r\n". * Added a check to header_parse() that effectively converts an "\r\n" in the input into '\n'. * Added a conditional so that header_parse() doesn't pass the empty string to header_save()---a behavior I observed in testing, at the end of a header block with "\r\n" line endings. * Simplified the last if(in_header) conditional in header_parse(), because it erroneously assumes that if in_header == True, then c could have some value other than EOF. (See the condition on the previous "while" loop, and the lack of any other way to exit said loop.) header_parse() will now properly grab a header if fed a message without a body (i.e. no "\n\n" ending the header block), although this code will still drop a header if there is no newline at the end. Christoph, thank you for your excellent analysis, and the test cases. I made use of them, and with my changes sSMTP appears to do the right thing. Index: ssmtp-2.64/ssmtp.c =================================================================== --- ssmtp-2.64.orig/ssmtp.c +++ ssmtp-2.64/ssmtp.c @@ -375,6 +375,12 @@ bool_t standardise(char *str, bool_t *li if((p = strchr(str, '\n'))) { *p = (char)NULL; *linestart = True; + + /* If the line ended in "\r\n", then drop the '\r' too */ + sl = strlen(str); + if(sl >= 1 && str[sl - 1] == '\r') { + str[sl - 1] = (char)NULL; + } } return(leadingdot); } @@ -768,6 +774,14 @@ void header_parse(FILE *stream) } len++; + if(l == '\r' && c == '\n') { + /* Properly handle input that already has "\r\n" + line endings; see https://bugs.debian.org/584162 */ + l = (len >= 2 ? *(q - 2) : '\n'); + q--; + len--; + } + if(l == '\n') { switch(c) { case ' ': @@ -790,7 +804,9 @@ void header_parse(FILE *stream) if((q = strrchr(p, '\n'))) { *q = (char)NULL; } - header_save(p); + if(len > 0) { + header_save(p); + } q = p; len = 0; @@ -800,35 +816,12 @@ void header_parse(FILE *stream) l = c; } - if(in_header) { - if(l == '\n') { - switch(c) { - case ' ': - case '\t': - /* Must insert '\r' before '\n's embedded in header - fields otherwise qmail won't accept our mail - because a bare '\n' violates some RFC */ - - *(q - 1) = '\r'; /* Replace previous \n with \r */ - *q++ = '\n'; /* Insert \n */ - len++; - - break; - - case '\n': - in_header = False; - - default: - *q = (char)NULL; - if((q = strrchr(p, '\n'))) { - *q = (char)NULL; - } - header_save(p); - - q = p; - len = 0; - } + if(in_header && l == '\n') { + /* Got EOF while reading the header */ + if((q = strrchr(p, '\n'))) { + *q = (char)NULL; } + header_save(p); } (void)free(p); }