]> Pileus Git - ~andy/fetchmail/commitdiff
SECURITY FIX: DoS on EILSEQ in report_*() in -vv and multibyte-locales.
authorMatthias Andree <matthias.andree@gmx.de>
Sun, 18 Apr 2010 16:01:38 +0000 (18:01 +0200)
committerMatthias Andree <matthias.andree@gmx.de>
Sun, 18 Apr 2010 16:06:35 +0000 (18:06 +0200)
Makefile.am
NEWS
fetchmail-SA-2010-02.txt [new file with mode: 0644]
rfc822.c
uid.c

index 900ea59e5a762be8a879835f3e094bd1d702e902..de4e4461f9252626a9515f1786e1ad1648d53439 100644 (file)
@@ -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 223811adc08fd27348cffc3938cda04110d91523..802309cfeb4898413964312ffb7aef2f97bc55e5 100644 (file)
--- 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 (file)
index 0000000..3e2e33b
--- /dev/null
@@ -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");
index 6f2dbf30fdf9503c0647a3a2c44fb7507f8dd4a1..dbcda322ad8471bd792e20ebb3b0e0566b08d780 100644 (file)
--- 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 fdc6f5da78e10e59c7de0275ab8378f2ba4132f5..d813beec6170307e5c8f59297117e1cd5eaeaba4 100644 (file)
--- 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");