Re: [Fetchmail-users] FM v6.4.20 - concatenated log lines

[Matthias A. <mat...@gm...>]

Am 08.08.21 um 21:54 schrieb Matthias Andree:
> Am 08.08.21 um 20:07 schrieb J.Edner:
>> Hi,
>> I've just compiled and installed Fetchmail 6.4.20 on my server and found
>> out, that some log lines are missing a new-line at the end, so that they
>> are concatenated as follows:
>>
>> fetchmail: 16 messages for XYZ at SERVERfetchmail: reading message
>> XYZ@SERVER:1 of 16fetchmail: reading message XYZ@SERVER:2 of
>> 16fetchmail: reading message XYZ@SERVER:3 of 16fetchmail: reading
>> message ...
>>
>> I checked the source code and saw that a new-line seems to be missing.
>> After I've applied this small patch all messages are correctly written
>> to the log file again:
>>
>> ----------
>> --- a/driver.c. 2021-08-08 17:06:53.756148421 +0200
>> +++ b/driver.c  2021-08-08 17:09:20.231666827 +0200
>> @@ -629,7 +629,7 @@ static int fetch_messages(int mailserver
>>
>>         if (outlevel > O_SILENT)
>>         {
>> -         report_build(stdout, GT_("reading message %s@%s:%d of %d"),
>> +         report_build(stdout, GT_("reading message %s@%s:%d of %d\n"),
>>                          ctl->remotename, ctl->server.truename,
>>                          num, count);
>> ----------
>>
>> The result looks as follows now again:
>>
>> fetchmail: 16 messages for XYZ at SERVER
>> fetchmail: reading message XYZ@SERVER:1 of 16
>> fetchmail: reading message XYZ@SERVER:2 of 16
>> fetchmail: reading message XYZ@SERVER:3 of 16
>> ...

Got it.

There is one statement in report_build, "if (n > 0)
partial_message_size_used += n;" which slipped outside the #if
defined(VA_START)...#endif block and that causes an excess (double)
increment of the string length; because both report_vbuild() and then
again this cited statement will bump the partial_message_size_used
counter. In effect, the buffer allocation is excessive and, more
importantly, the 2nd and later report_build() before the next report()
or report_complete() write too deep into the buffer. This will not cause
overruns due to the reallocation prior to the vsnprintf/sprintf, but it
write starts behind the '\0' byte, instead of right over it, so the
string also gets truncated to the first fragment written with
report_vbuild().

This does not affect --syslog or console output so, for lack of relevant
test items, escaped my testing. Sorry about that.

This patch fixes the issue (this is for perusal, I am attaching a copy,
or if it doesn't make it through to the list, cherry-pick the report.c
part from Git commit d3db2da1). fetchmail 6.4.21 and revised security
announcement coming up, and 6.5.0-beta5 should not be too far out either.

https://gitlab.com/fetchmail/fetchmail/-/commit/d3db2da1d13bd2419370ad96defb92eecb17064c
https://sourceforge.net/p/fetchmail/git/ci/d3db2da1d13bd2419370ad96defb92eecb17064c/

diff --git a/report.c b/report.c
index aea6b3ea..2db7d0a9 100644
--- a/report.c
+++ b/report.c
@@ -286,10 +286,11 @@ report_build (FILE *errfp, message, va_alist)
     n = snprintf (partial_message + partial_message_size_used,
             partial_message_size - partial_message_size_used,
             message, a1, a2, a3, a4, a5, a6, a7, a8);
-#endif
 
     if (n > 0) partial_message_size_used += n;
 
+#endif
+
     if (unbuffered && partial_message_size_used != 0)
     {
     partial_message_size_used = 0;