Re: More about UW c-client library

["Jaldhar H. Vyas" <jaldhar () debian org>]

On Sat, 2 Sep 2000, Juhapekka Tolvanen wrote:

> Here is more information about that bug.
>
> http://cgi.debian.org/cgi-bin/bugreport.cgi?archive=no&bug=70647
>
> It seems, that they will have some patch real soon:
>
> (CLIP HERE)
>
> > Upon a quick glance, there indeed appears to be no checks at all
> > for buffer overflows. A buf of 8k is allocated into which the
> > From:, Status:, X-Status, and X-Keywords: headers are placed,
> > with simple
> >
> >       sprintf (buf + strlen (buf),"...
> >
> > commands. So having extremely long X-Keywords in mail messages
> > will screw things up. Double yuck.
> >
> > This is in imap-4.7c/src/osdep/unix/unix.c BTW.
> >
> > See the original message and the accompanying thread in debian-devel,
> > archive/latest/67244 , Message-ID <39AD820C.6AD0818C () axis com> from
> > Cristian Ionescu-Idbohrn <cii () axis com>
>
> Ok, I've patched unix.c to use snprintf(3) instead of sprintf(3).  This
> is
> only the tip of the iceberg however.  There is a source code scanner
> called its4 which checks for unsafe coding practices and I ran it on
> imapd.  The report was about a mile long :(
>
> (CLIP HERE)

Juhapekka has jumped the gun a bit.  I've just uploaded an updated package
to Debian now.  Here's the patch.  *BUT* I don't know if this is a
complete fix to the problem or what the exact scope of the problem
is. (longtime BugTraq readers know all about the imap source :-)  We're
still testing and this could be wrong.  I'd appreciate any feedback.


Once again, this is in src/osdep/unix/unix.c.


--- unix.c.orig Thu Aug 31 11:09:25 2000
+++ unix.c      Fri Sep  1 16:11:30 2000
@@ -235,7 +235,7 @@
   int i,fd;
   time_t ti = time (0);
   if (!(s = dummy_file (mbx,mailbox))) {
-    sprintf (tmp,"Can't create %.80s: invalid name",mailbox);
+    snprintf (tmp,MAILTMPLEN,"Can't create %.80s: invalid name",mailbox);
     mm_log (tmp,ERROR);
   }
                                /* create underlying file */
@@ -244,7 +244,7 @@
     if ((s = strrchr (s,'/')) && !s[1]) return T;
     if ((fd = open (mbx,O_WRONLY,
                    (int) mail_parameters (NIL,GET_MBXPROTECTION,NIL))) < 0) {
-      sprintf (tmp,"Can't reopen mailbox node %.80s: %s",mbx,strerror (errno));
+      snprintf (tmp,MAILTMPLEN,"Can't reopen mailbox node %.80s: %s",mbx,strerror (errno));
       mm_log (tmp,ERROR);
       unlink (mbx);            /* delete the file */
     }
@@ -252,18 +252,18 @@
     else if (mail_parameters (NIL,GET_USERHASNOLIFE,NIL)) ret = T;
     else {                     /* initialize header */
       memset (tmp,'\0',MAILTMPLEN);
-      sprintf (tmp,"From %s %sDate: ",pseudo_from,ctime (&ti));
+      snprintf (tmp,MAILTMPLEN,"From %s %sDate: ",pseudo_from,ctime (&ti));
       rfc822_fixed_date (s = tmp + strlen (tmp));
                                /* write the pseudo-header */
-      sprintf (s += strlen (s),
+      snprintf (s += strlen (s),MAILTMPLEN-strlen(tmp),
               "\nFrom: %s <%s@%s>\nSubject: %s\nX-IMAP: %010lu 0000000000",
               pseudo_name,pseudo_from,mylocalhost (),pseudo_subject,
               (unsigned long) ti);
       for (i = 0; i < NUSERFLAGS; ++i) if (default_user_flag (i))
-       sprintf (s += strlen (s)," %s",default_user_flag (i));
-      sprintf (s += strlen (s),"\nStatus: RO\n\n%s\n\n",pseudo_msg);
+       snprintf (s += strlen (s),MAILTMPLEN-strlen(tmp)," %s",default_user_flag (i));
+      snprintf (s += strlen
(s),MAILTMPLEN-strlen(tmp),"\nStatus: RO\n\n%s\n\n",pseudo_msg);
       if ((write (fd,tmp,strlen (tmp)) < 0) || close (fd)) {
-       sprintf (tmp,"Can't initialize mailbox node %.80s: %s",mbx,
+       snprintf (tmp,MAILTMPLEN-strlen(tmp),"Can't initialize mailbox node %.80s: %s",mbx,
                 strerror (errno));
        mm_log (tmp,ERROR);
        unlink (mbx);           /* delete the file */