Fix various buffer overruns in fmt_scan.c; the bulk of this is passing

buffer length through to decode_rfc2047() and having that function do
sufficient bookkeeping to avoid running off the end of the buffer.

diff --git a/ChangeLog b/ChangeLog
index 6d2553e..ca22c28 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2005-11-06  Peter Maydell  <pmaydell@chiark.greenend.org.uk>
 
+       * sbr/fmt_rfc2047.c, sbr/fmt_scan.c, h/prototypes.h: fix various
+       possible overruns of the buffers in fmt_scan() which would cause
+       crashes if scan was run with '-width 16536' or similar.
+
        * uip/popsbr.c: fix compile error which only showed up if nmh
        was configured with --enable-apop.
 

diff --git a/h/prototypes.h b/h/prototypes.h
index edddc47..21c15a7 100644 (file)
--- a/h/prototypes.h
+++ b/h/prototypes.h
@@ -46,7 +46,7 @@ char *copy (char *, char *);
 char **copyip (char **, char **, int);
 void cpydata (int, int, char *, char *);
 void cpydgst (int, int, char *, char *);
-int decode_rfc2047 (char *, char *);
+int decode_rfc2047 (char *, char *, size_t);
 void discard (FILE *);
 int done (int);
 int ext_hook(char *, char *, char *);

diff --git a/sbr/fmt_rfc2047.c b/sbr/fmt_rfc2047.c
index a87fc0e..9f5b26d 100644 (file)
--- a/sbr/fmt_rfc2047.c
+++ b/sbr/fmt_rfc2047.c
@@ -55,8 +55,11 @@ unqp (unsigned char byte1, unsigned char byte2)
  * Decode the string as a RFC-2047 header field
  */
 
+/* Add character to the destination buffer, and bomb out if it fills up */
+#define ADDCHR(C) do { *q++ = (C); dstlen--; if (!dstlen) goto buffull; } while (0)
+
 int
-decode_rfc2047 (char *str, char *dst) 
+decode_rfc2047 (char *str, char *dst, size_t dstlen)
 {
     char *p, *q, *pp;
     char *startofmime, *endofmime;
@@ -68,8 +71,9 @@ decode_rfc2047 (char *str, char *dst)
 #ifdef HAVE_ICONV
     int use_iconv = 0;          /* are we converting encoding with iconv? */
     iconv_t cd;
-    int fromutf8;
-    char *saveq, *convbuf;
+    int fromutf8 = 0;
+    char *saveq, *convbuf = NULL;
+    size_t savedstlen;
 #endif
 
     if (!str)
@@ -96,7 +100,7 @@ decode_rfc2047 (char *str, char *dst)
         * last iteration, then add it first.
         */
        if (equals_pending) {
-           *q++ = '=';
+           ADDCHR('=');
            equals_pending = 0;
            between_encodings = 0;      /* we have added non-whitespace text */
        }
@@ -107,7 +111,7 @@ decode_rfc2047 (char *str, char *dst)
                whitespace++;
            else
                between_encodings = 0;  /* we have added non-whitespace text */
-           *q++ = *p;
+           ADDCHR(*p);
            continue;
        }
 
@@ -185,15 +189,25 @@ decode_rfc2047 (char *str, char *dst)
             * We will roll back the buffer the number of whitespace
             * characters we've seen since last encoded word.
             */
-           if (between_encodings)
+           if (between_encodings) {
                q -= whitespace;
+               dstlen += whitespace;
+           }
 
 #ifdef HAVE_ICONV
            if (use_iconv) {
-               saveq = q;
+               saveq = q;
+               savedstlen = dstlen;
                if (!(q = convbuf = (char *)malloc(endofmime - startofmime)))
                    continue;
             }
+/* ADDCHR2 is for adding characters when q is or might be convbuf:
+ * in this case on buffer-full we want to run iconv before returning.
+ * I apologise for the dreadful name.
+ */
+#define ADDCHR2(C) do { *q++ = (C); dstlen--; if (!dstlen) goto iconvbuffull; } while (0)
+#else
+#define ADDCHR2(C) ADDCHR(C)
 #endif
 
            /* Now decode the text */
@@ -207,9 +221,9 @@ decode_rfc2047 (char *str, char *dst)
                            *q++ = c;
                        pp += 2;
                    } else if (*pp == '_') {
-                       *q++ = ' ';
+                       ADDCHR2(' ');
                    } else {
-                       *q++ = *pp;
+                       ADDCHR2(*pp);
                    }
                }
            } else {
@@ -231,7 +245,7 @@ decode_rfc2047 (char *str, char *dst)
                        pp++;
                    }
                    if (pp < endofmime && c1 != -1 && c2 != -1) {
-                       *q++ = (c1 << 2) | (c2 >> 4);
+                       ADDCHR2((c1 << 2) | (c2 >> 4));
                        pp++;
                    }
                    /* 4 + 4 bits */
@@ -240,7 +254,7 @@ decode_rfc2047 (char *str, char *dst)
                        pp++;
                    }
                    if (pp < endofmime && c2 != -1 && c3 != -1) {
-                       *q++ = ((c2 & 0xF) << 4) | (c3 >> 2);
+                       ADDCHR2(((c2 & 0xF) << 4) | (c3 >> 2));
                        pp++;
                    }
                    /* 2 + 6 bits */
@@ -249,37 +263,54 @@ decode_rfc2047 (char *str, char *dst)
                        pp++;
                    }
                    if (pp < endofmime && c3 != -1 && c4 != -1) {
-                       *q++ = ((c3 & 0x3) << 6) | (c4);
+                       ADDCHR2(((c3 & 0x3) << 6) | (c4));
                        pp++;
                    }
                }
            }
 
 #ifdef HAVE_ICONV
+       iconvbuffull:
+           /* NB that the string at convbuf is not necessarily NUL terminated here:
+            * q points to the first byte after the valid part.
+            */
             /* Convert to native character set */
            if (use_iconv) {
                size_t inbytes = q - convbuf;
-               size_t outbytes = BUFSIZ;
                ICONV_CONST char *start = convbuf;
                
                while (inbytes) {
-                   if (iconv(cd, &start, &inbytes, &saveq, &outbytes) ==
+                   if (iconv(cd, &start, &inbytes, &saveq, &savedstlen) ==
                            (size_t)-1) {
                        if (errno != EILSEQ) break;
                        /* character couldn't be converted. we output a `?'
                         * and try to carry on which won't work if
                         * either encoding was stateful */
-                       iconv (cd, 0, 0, &saveq, &outbytes);
+                       iconv (cd, 0, 0, &saveq, &savedstlen);
+                       if (!savedstlen)
+                           break;
                        *saveq++ = '?';
-                        /* skip to next input character */
+                       savedstlen--;
+                       if (!savedstlen)
+                           break;
+                       /* skip to next input character */
                        if (fromutf8) {
-                           for (start++;(*start & 192) == 128;start++)
+                           for (start++;(start < q) && ((*start & 192) == 128);start++)
                                inbytes--;
                        } else
                            start++, inbytes--;
+                       if (start >= q)
+                           break;
                    }
                }
                q = saveq;
+               /* Stop now if (1) we hit the end of the buffer trying to do
+                * MIME decoding and have just iconv-converted a partial string
+                * or (2) our iconv-conversion hit the end of the buffer.
+                */
+               if (!dstlen || !savedstlen)
+                   goto buffull;
+               dstlen = savedstlen;
                free(convbuf);
            }
 #endif
@@ -301,8 +332,14 @@ decode_rfc2047 (char *str, char *dst)
 
     /* If an equals was pending at end of string, add it now. */
     if (equals_pending)
-       *q++ = '=';
+       ADDCHR('=');
     *q = '\0';
 
     return encoding_found;
+
+  buffull:
+    /* q is currently just off the end of the buffer, so rewind to NUL terminate */
+    q--;
+    *q = '\0';
+    return encoding_found;
 }

diff --git a/sbr/fmt_scan.c b/sbr/fmt_scan.c
index 7558eca..8f81e77 100644 (file)
--- a/sbr/fmt_scan.c
+++ b/sbr/fmt_scan.c
@@ -442,14 +442,14 @@ fmt_scan (struct format *format, char *scanl, int width, int *dat)
            break;
 
        case FT_LS_DECODECOMP:
-           if (decode_rfc2047(fmt->f_comp->c_text, buffer2))
+           if (decode_rfc2047(fmt->f_comp->c_text, buffer2, sizeof(buffer2)))
                str = buffer2;
            else
                str = fmt->f_comp->c_text;
            break;
 
        case FT_LS_DECODE:
-           if (str && decode_rfc2047(str, buffer2))
+           if (str && decode_rfc2047(str, buffer2, sizeof(buffer2)))
                str = buffer2;
            break;
 
@@ -458,6 +458,7 @@ fmt_scan (struct format *format, char *scanl, int width, int *dat)
                    char *xp;
 
                    strncpy(buffer, str, sizeof(buffer));
+                   buffer[sizeof(buffer)-1] = '\0';
                    str = buffer;
                    while (isspace(*str))
                            str++;
@@ -646,6 +647,7 @@ fmt_scan (struct format *format, char *scanl, int width, int *dat)
            if ((str = mn->m_pers) == NULL) {
                if ((str = mn->m_note)) {
                    strncpy (buffer, str, sizeof(buffer));
+                   buffer[sizeof(buffer)-1] = '\0';
                    str = buffer;
                    if (*str == '(')
                        str++;
@@ -690,6 +692,8 @@ fmt_scan (struct format *format, char *scanl, int width, int *dat)
            if (str) {          
                int m;
                strncpy(buffer, str, sizeof(buffer));
+               /* strncpy doesn't NUL-terminate if it fills the buffer */
+               buffer[sizeof(buffer)-1] = '\0';
                str = buffer;
        
                /* we will parse from buffer to buffer2 */