SECURITY FIX: DoS on EILSEQ in report_*() in -vv and multibyte-locales.

5 files changed, 245 insertions, 12 deletions

diff --git a/Makefile.am b/Makefile.am
index 900ea59e..de4e4461 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -126,6 +126,7 @@ DISTDOCS=	FAQ FEATURES NOTES OLDNEWS fetchmail-man.html \
 		fetchmail-features.html README.SSL README.NTLM \
 		README.packaging README.SSL-SERVER \
 		fetchmail-FAQ.book fetchmail-FAQ.pdf fetchmail-FAQ.html \
+		fetchmail-SA-2010-02.txt \
 		fetchmail-SA-2010-01.txt \
 		fetchmail-SA-2009-01.txt \
 		fetchmail-SA-2008-01.txt \
diff --git a/NEWS b/NEWS
index 223811ad..802309cf 100644
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,14 @@ removed from a 6.4.0 or newer release.)
 
 fetchmail-6.3.17 (not yet released):
 
+# SECURITY FIX
+* Fetchmail before release 6.3.17 did not properly sanitize external input
+  (mail headers and UID). When a multi-character locale (such as UTF-8) was in use, 
+  this could cause memory exhaustion and thus a denial of service, because
+  fetchmail's report.c functions assumed that non-success of [v]snprintf was
+  due to insufficient buffer size allocation. It would then repeatedly reallocate
+  a larger buffer and fail formatting again. See fetchmail-SA-2010-02.txt.
+
 # REGRESSION FIX
 * Fix string handling in rcfile scanner, which caused fetchmail to misparse a
   run control file in certain circumstances.  Fixes BerliOS bug #14257.
diff --git a/fetchmail-SA-2010-02.txt b/fetchmail-SA-2010-02.txt
new file mode 100644
index 00000000..3e2e33bd
--- /dev/null
+++ b/fetchmail-SA-2010-02.txt
@@ -0,0 +1,209 @@
+- DRAFT - XXX - DRAFT -
+
+fetchmail-SA-2010-02: Denial of service in debug mode w/ multichar locales
+
+Topics:		Denial of service in debug output.
+
+Author:		Matthias Andree
+Version:	0.1 XXX
+Announced:	XXX
+Type:		malloc() Buffer overrun with printable characters
+Impact:		Denial of service.
+Danger:		low
+
+CVE Name:	CVE-2010-XXXX
+CVSSv2:		XXX
+URL:		http://www.fetchmail.info/fetchmail-SA-2010-02.txt
+Project URL:	http://www.fetchmail.info/
+
+Affects:	fetchmail releases 4.6.3 up to and including 6.3.16
+
+Not affected:	fetchmail release 6.3.17 and newer
+
+Corrected:	2010-04-18 Git (XXX)
+
+
+0. Release history
+==================
+
+2010-04-18 0.1	first draft (visible in SVN and through oss-security)
+XXX
+
+
+1. Background
+=============
+
+fetchmail is a software package to retrieve mail from remote POP2, POP3,
+IMAP, ETRN or ODMR servers and forward it to local SMTP, LMTP servers or
+message delivery agents. It supports SSL and TLS security layers through
+the OpenSSL library, if enabled at compile time and if also enabled at
+run time.
+
+
+2. Problem description and Impact
+=================================
+
+In debug mode (-v -v), fetchmail prints information that was obtained from the
+upstream server (POP3 UIDL lists) or from message headers retrieved from it.
+  If printing such information fails, for instance because there are invalid
+multibyte character sequences in this information (message headers), fetchmail
+will misinterpret this condition, and believe that the buffer was too small,
+and reallocate a bigger one (with linearly increasing buffer size), and repeat,
+until the allocation fails. At that point, fetchmail will abort.
+
+Note that the "Affects:" line above may be inaccurate, and it may be that
+versions before 5.6.6 are actually unaffected.  The author was unable to
+compile such old fetchmail versions to verify the existence of the bug.
+  Given that other security issues are present in such versions, those should
+not be used, and the wider version range was listed as vulnerable to err
+towards the safe.
+
+
+3. Solution
+===========
+
+There are two alternatives, either of them by itself is sufficient:
+
+a. Apply the patch found in section B of this announcement to
+   fetchmail 6.3.14 or newer, recompile and reinstall it.
+
+b. Install fetchmail 6.3.17 or newer after it will have become available.
+   The fetchmail source code is always available from
+   <http://developer.berlios.de/project/showfiles.php?group_id=1824>.
+
+
+4. Workaround
+=============
+
+Run fetchmail with at most one -v (--verbose) option.
+
+
+A. Copyright, License and Warranty
+==================================
+
+(C) Copyright 2010 by Matthias Andree, <matthias.andree@gmx.de>.
+Some rights reserved.
+
+This work is licensed under the Creative Commons
+Attribution-Noncommercial-No Derivative Works 3.0 Germany License.
+To view a copy of this license, visit
+http://creativecommons.org/licenses/by-nc-nd/3.0/de/ or send a letter to
+
+Creative Commons
+171 Second Street
+Suite 300
+SAN FRANCISCO, CALIFORNIA 94105
+USA
+
+
+THIS WORK IS PROVIDED FREE OF CHARGE AND WITHOUT ANY WARRANTIES.
+Use the information herein at your own risk.
+
+
+B. Patch to remedy the problem
+==============================
+
+Note that when taking this from a GnuPG clearsigned file, the lines
+starting with a "-" character are prefixed by another "- " (dash +
+blank) combination. Either feed this file through GnuPG to strip them,
+or strip them manually. You may want to use the "-p1" flag to patch.
+
+Whitespace differences can usually be ignored by invoking "patch -l",
+so try this if the patch does not apply.
+
+diff --git a/rfc822.c b/rfc822.c
+index 6f2dbf3..dbcda32 100644
+--- a/rfc822.c
++++ b/rfc822.c
+@@ -25,6 +25,7 @@ MIT license.  Compile with -DMAIN to build the demonstrator.
+ #include  <stdlib.h>
+ 
+ #include "fetchmail.h"
++#include "sdump.h"
+ 
+ #ifndef MAIN
+ #include "i18n.h"
+@@ -74,9 +75,10 @@ char *reply_hack(
+     }
+ 
+ #ifndef MAIN
+-    if (outlevel >= O_DEBUG)
+-	report_build(stdout, GT_("About to rewrite %.*s...\n"),
+-			(int)BEFORE_EOL(buf), buf);
++    if (outlevel >= O_DEBUG) {
++	report_build(stdout, GT_("About to rewrite %s...\n"), (cp = sdump(buf, BEFORE_EOL(buf))));
++	xfree(cp);
++    }
+ 
+     /* make room to hack the address; buf must be malloced */
+     for (cp = buf; *cp; cp++)
+@@ -211,9 +213,12 @@ char *reply_hack(
+     }
+ 
+ #ifndef MAIN
+-    if (outlevel >= O_DEBUG)
+-	report_complete(stdout, GT_("...rewritten version is %.*s.\n"),
+-			(int)BEFORE_EOL(buf), buf);
++    if (outlevel >= O_DEBUG) {
++	report_complete(stdout, GT_("...rewritten version is %s.\n"),
++			(cp = sdump(buf, BEFORE_EOL(buf))));
++	xfree(cp)
++    }
++
+ #endif /* MAIN */
+     *length = strlen(buf);
+     return(buf);
+diff --git a/uid.c b/uid.c
+index fdc6f5d..d813bee 100644
+--- a/uid.c
++++ b/uid.c
+@@ -20,6 +20,7 @@
+ 
+ #include "fetchmail.h"
+ #include "i18n.h"
++#include "sdump.h"
+ 
+ /*
+  * Machinery for handling UID lists live here.  This is mainly to support
+@@ -260,8 +261,11 @@ void initialize_saved_lists(struct query *hostlist, const char *idfile)
+ 	if (uidlcount)
+ 	{
+ 	    report_build(stdout, GT_("Scratch list of UIDs:"));
+-	    for (idp = scratchlist; idp; idp = idp->next)
+-		report_build(stdout, " %s", idp->id);
++	    for (idp = scratchlist; idp; idp = idp->next) {
++		char *t = sdump(idp->id, strlen(idp->id));
++		report_build(stdout, " %s", t);
++		free(t);
++	    }
+ 	    if (!idp)
+ 		report_build(stdout, GT_(" <empty>"));
+ 	    report_complete(stdout, "\n");
+@@ -517,8 +521,11 @@ void uid_swap_lists(struct query *ctl)
+ 	    report_build(stdout, GT_("Merged UID list from %s:"), ctl->server.pollname);
+ 	else
+ 	    report_build(stdout, GT_("New UID list from %s:"), ctl->server.pollname);
+-	for (idp = dofastuidl ? ctl->oldsaved : ctl->newsaved; idp; idp = idp->next)
+-	    report_build(stdout, " %s = %d", idp->id, idp->val.status.mark);
++	for (idp = dofastuidl ? ctl->oldsaved : ctl->newsaved; idp; idp = idp->next) {
++	    char *t = sdump(idp->id, strlen(idp->id));
++	    report_build(stdout, " %s = %d", t, idp->val.status.mark);
++	    free(t);
++        }
+ 	if (!idp)
+ 	    report_build(stdout, GT_(" <empty>"));
+ 	report_complete(stdout, "\n");
+@@ -567,8 +574,11 @@ void uid_discard_new_list(struct query *ctl)
+ 	/* this is now a merged list! the mails which were seen in this
+ 	 * poll are marked here. */
+ 	report_build(stdout, GT_("Merged UID list from %s:"), ctl->server.pollname);
+-	for (idp = ctl->oldsaved; idp; idp = idp->next)
+-	    report_build(stdout, " %s = %d", idp->id, idp->val.status.mark);
++	for (idp = ctl->oldsaved; idp; idp = idp->next) {
++	    char *t = sdump(idp->id, strlen(idp->id));
++	    report_build(stdout, " %s = %d", t, idp->val.status.mark);
++	    free(t);
++	}
+ 	if (!idp)
+ 	    report_build(stdout, GT_(" <empty>"));
+ 	report_complete(stdout, "\n");
diff --git a/rfc822.c b/rfc822.c
index 6f2dbf30..dbcda322 100644
--- a/rfc822.c
+++ b/rfc822.c
@@ -25,6 +25,7 @@ MIT license.  Compile with -DMAIN to build the demonstrator.
 #include  <stdlib.h>
 
 #include "fetchmail.h"
+#include "sdump.h"
 
 #ifndef MAIN
 #include "i18n.h"
@@ -74,9 +75,10 @@ char *reply_hack(
     }
 
 #ifndef MAIN
-    if (outlevel >= O_DEBUG)
-	report_build(stdout, GT_("About to rewrite %.*s...\n"),
-			(int)BEFORE_EOL(buf), buf);
+    if (outlevel >= O_DEBUG) {
+	report_build(stdout, GT_("About to rewrite %s...\n"), (cp = sdump(buf, BEFORE_EOL(buf))));
+	xfree(cp);
+    }
 
     /* make room to hack the address; buf must be malloced */
     for (cp = buf; *cp; cp++)
@@ -211,9 +213,12 @@ char *reply_hack(
     }
 
 #ifndef MAIN
-    if (outlevel >= O_DEBUG)
-	report_complete(stdout, GT_("...rewritten version is %.*s.\n"),
-			(int)BEFORE_EOL(buf), buf);
+    if (outlevel >= O_DEBUG) {
+	report_complete(stdout, GT_("...rewritten version is %s.\n"),
+			(cp = sdump(buf, BEFORE_EOL(buf))));
+	xfree(cp)
+    }
+
 #endif /* MAIN */
     *length = strlen(buf);
     return(buf);
diff --git a/uid.c b/uid.c
index fdc6f5da..d813beec 100644
--- a/uid.c
+++ b/uid.c
@@ -20,6 +20,7 @@
 
 #include "fetchmail.h"
 #include "i18n.h"
+#include "sdump.h"
 
 /*
  * Machinery for handling UID lists live here.  This is mainly to support
@@ -260,8 +261,11 @@ void initialize_saved_lists(struct query *hostlist, const char *idfile)
 	if (uidlcount)
 	{
 	    report_build(stdout, GT_("Scratch list of UIDs:"));
-	    for (idp = scratchlist; idp; idp = idp->next)
-		report_build(stdout, " %s", idp->id);
+	    for (idp = scratchlist; idp; idp = idp->next) {
+		char *t = sdump(idp->id, strlen(idp->id));
+		report_build(stdout, " %s", t);
+		free(t);
+	    }
 	    if (!idp)
 		report_build(stdout, GT_(" <empty>"));
 	    report_complete(stdout, "\n");
@@ -517,8 +521,11 @@ void uid_swap_lists(struct query *ctl)
 	    report_build(stdout, GT_("Merged UID list from %s:"), ctl->server.pollname);
 	else
 	    report_build(stdout, GT_("New UID list from %s:"), ctl->server.pollname);
-	for (idp = dofastuidl ? ctl->oldsaved : ctl->newsaved; idp; idp = idp->next)
-	    report_build(stdout, " %s = %d", idp->id, idp->val.status.mark);
+	for (idp = dofastuidl ? ctl->oldsaved : ctl->newsaved; idp; idp = idp->next) {
+	    char *t = sdump(idp->id, strlen(idp->id));
+	    report_build(stdout, " %s = %d", t, idp->val.status.mark);
+	    free(t);
+        }
 	if (!idp)
 	    report_build(stdout, GT_(" <empty>"));
 	report_complete(stdout, "\n");
@@ -567,8 +574,11 @@ void uid_discard_new_list(struct query *ctl)
 	/* this is now a merged list! the mails which were seen in this
 	 * poll are marked here. */
 	report_build(stdout, GT_("Merged UID list from %s:"), ctl->server.pollname);
-	for (idp = ctl->oldsaved; idp; idp = idp->next)
-	    report_build(stdout, " %s = %d", idp->id, idp->val.status.mark);
+	for (idp = ctl->oldsaved; idp; idp = idp->next) {
+	    char *t = sdump(idp->id, strlen(idp->id));
+	    report_build(stdout, " %s = %d", t, idp->val.status.mark);
+	    free(t);
+	}
 	if (!idp)
 	    report_build(stdout, GT_(" <empty>"));
 	report_complete(stdout, "\n");