patch: qpopper (plugs another hole too)

[miquels () CISTRON NL (Miquel van Smoorenburg)]

This afternoon I sent a fix for qpopper. There have been some other patches
on the list too, but nobody except me noticed the X-UIDL problem.

On further careful inspection of all sprintf() functions in qpopper,
I think most of them are harmless. The UIDL is extracted from a buffer
1024 bytes big, minus X-UIDL:<space> so can never be any bigger then
1016 characters. That prevents most sprintf()s from overflowing since they
are printing in a 1024 byte buffer.

However I did find a few places in pop_uidl.c where an overflow _is_
possible because both data from the X-UIDL header and the from line are
combined in one sprintf().

Here's a new patch that addresses the X-UIDL bug and the original bugs.
Against qpopper-2.3 (the last free version) again.

PS This is the patch Debian qpopper-2.2 (bo) and qpopper-2.3 (hamm)
will use - I'm uploading them to master.debian.org at this very moment.

diff -ruN qpopper-2.3.orig/pop_log.c qpopper-2.3/pop_log.c
--- qpopper-2.3.orig/pop_log.c  Sat Mar 29 05:30:36 1997
+++ qpopper-2.3/pop_log.c       Sat Jun 27 23:26:39 1998
@@ -18,7 +18,11 @@
  *  log:    Make a log entry
  */

+#ifdef HAVE_VSNPRINTF
 static char msgbuf[MAXLINELEN];
+#else
+static char msgbuf[MAXLINELEN*4];
+#endif

 pop_log(va_alist)
 va_dcl
@@ -46,6 +50,9 @@
     arg6 = va_arg(ap, char *);
 #endif

+#ifdef HAVE_VSNPRINTF
+        vsnprintf(msgbuf,sizeof(msgbuf),format,ap);
+#else
 #ifdef HAVE_VSPRINTF
         vsprintf(msgbuf,format,ap);
 #else
@@ -57,6 +64,7 @@
 # endif
     va_end(ap);
 #endif
+#endif

     if (p->debug && p->trace) {
        clock = time(0);
@@ -67,6 +75,8 @@
         (void)fflush(p->trace);
     }
     else {
+       /* Protect syslog from too long messages */
+       if (strlen(msgbuf) >= 512) msgbuf[511] = 0;
         syslog (stat,"%s",msgbuf);
     }

diff -ruN qpopper-2.3.orig/pop_msg.c qpopper-2.3/pop_msg.c
--- qpopper-2.3.orig/pop_msg.c  Sat Mar 29 05:30:36 1997
+++ qpopper-2.3/pop_msg.c       Sat Jun 27 23:26:39 1998
@@ -34,7 +34,11 @@
 #ifdef PYRAMID
     char           *   arg1, *arg2, *arg3, *arg4, *arg5, *arg6;
 #endif
+#ifdef HAVE_VSNPRINTF
     char                message[MAXLINELEN];
+#else
+    char                message[MAXLINELEN * 4];
+#endif

     va_start(ap);
     p = va_arg(ap, POP *);
@@ -63,6 +67,9 @@

     /*  Append the message (formatted, if necessary) */
     if (format)
+#ifdef HAVE_VSNPRINTF
+        vsnprintf(mp,sizeof(message) - strlen(mp) - 3, format,ap);
+#else
 #ifdef HAVE_VSPRINTF
         vsprintf(mp,format,ap);
 #else
@@ -72,6 +79,7 @@
         (void)sprintf(mp,format,((int *)ap)[0],((int *)ap)[1],((int *)ap)[2],
                 ((int *)ap)[3],((int *)ap)[4]);
 # endif
+#endif
 #endif
     va_end(ap);

diff -ruN qpopper-2.3.orig/pop_uidl.c qpopper-2.3/pop_uidl.c
--- qpopper-2.3.orig/pop_uidl.c Sat Mar 29 05:30:36 1997
+++ qpopper-2.3/pop_uidl.c      Sat Jun 27 23:31:05 1998
@@ -54,7 +54,7 @@
                            "Message %d has been marked for deletion.",msg_id));
       } else {

-       sprintf(buffer, "%d %s", msg_id, mp->uidl_str);
+       sprintf(buffer, "%d %.900s", msg_id, mp->uidl_str);
         if (nl = index(buffer, NEWLINE)) *nl = 0;
        return (pop_msg (p,POP_SUCCESS, buffer));
       }
@@ -70,7 +70,7 @@
            /*  Is the message flagged for deletion? */
            if (mp->del_flag) continue;

-           sprintf(buffer, "%d %s", x, mp->uidl_str);
+           sprintf(buffer, "%d %.900s", x, mp->uidl_str);
 /*         nl = index(mp->uidl_str, NEWLINE); */
            pop_sendline(p, buffer);
 /*
@@ -147,9 +147,9 @@
                            "Message %d has been marked for deletion.",msg_id));
       } else {

-       sprintf(buffer, "%d %s", msg_id, mp->uidl_str);
+       sprintf(buffer, "%d %.900s", msg_id, mp->uidl_str);
         if (nl = index(buffer, NEWLINE)) *nl = 0;
-       sprintf(buffer, "%s %d %s", buffer, mp->length, from_hdr(p, mp));
+       sprintf(buffer, "%s %d %.100s", buffer, mp->length, from_hdr(p, mp));
        return (pop_msg (p,POP_SUCCESS, buffer));
       }
     } else {
@@ -164,9 +164,9 @@
            /*  Is the message flagged for deletion? */
            if (mp->del_flag) continue;

-           sprintf(buffer, "%d %s", x, mp->uidl_str);
+           sprintf(buffer, "%d %.900s", x, mp->uidl_str);
            if (nl = index(buffer, NEWLINE)) *nl = 0;
-           sprintf(buffer, "%s %d %s", buffer, mp->length, from_hdr(p, mp));
+           sprintf(buffer, "%s %d %.100s", buffer, mp->length, from_hdr(p, mp));
            pop_sendline(p, buffer);
         }
     }
diff -ruN qpopper-2.3.orig/popper.h qpopper-2.3/popper.h
--- qpopper-2.3.orig/popper.h   Mon Mar 31 22:10:18 1997
+++ qpopper-2.3/popper.h        Sat Jun 27 23:26:39 1998
@@ -128,6 +128,7 @@
 #endif

 #ifdef LINUX
+# define HAVE_VSNPRINTF
 # define POP_MAILDIR "/var/spool/mail"
 # define POP_DROP    "/var/spool/mail/.%s.pop"
 # define POP_TMPDROP "/var/spool/mail/tmpXXXXXX"
--
 Miquel van Smoorenburg | Our vision is to speed up time,
    miquels () cistron nl  |   eventually eliminating it.