[Nmh-commits] nmh ChangeLog mts/smtp/smtp.c mts/smtp/smtp.h

[Peter Maydell]

CVSROOT:        /cvsroot/nmh
Module name:    nmh
Changes by:     Peter Maydell <pm215>   08/04/29 21:05:30

Modified files:
        .              : ChangeLog 
        mts/smtp       : smtp.c smtp.h 

Log message:
        Correct various places in smtp.c where the reply string might not have 
been
        correctly NUL-terminated. Includes a fix for a particularly nasty and 
long
        standing screwup where the buffer length counting in smhear() was 
totally
        broken for continued lines from the server.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/nmh/ChangeLog?cvsroot=nmh&r1=1.260&r2=1.261
http://cvs.savannah.gnu.org/viewcvs/nmh/mts/smtp/smtp.c?cvsroot=nmh&r1=1.22&r2=1.23
http://cvs.savannah.gnu.org/viewcvs/nmh/mts/smtp/smtp.h?cvsroot=nmh&r1=1.2&r2=1.3

Patches:
Index: ChangeLog
===================================================================
RCS file: /cvsroot/nmh/nmh/ChangeLog,v
retrieving revision 1.260
retrieving revision 1.261
diff -u -b -r1.260 -r1.261
--- ChangeLog   29 Apr 2008 20:59:04 -0000      1.260
+++ ChangeLog   29 Apr 2008 21:05:30 -0000      1.261
@@ -8,6 +8,12 @@
        (the SASL libraries now care if you pass in the wrong
        length).
 
+       * Correct various places in smtp.c where the reply string
+       might not have been correctly NUL-terminated. Includes a
+       fix for a particularly nasty and long standing screwup
+       where the buffer length counting in smhear() was totally
+       broken for continued lines from the server.
+
 2008-04-29  Peter Maydell  <address@hidden>
 
        * Cope with sasl_decode64() returning SASL_CONTINUE, as

Index: mts/smtp/smtp.c
===================================================================
RCS file: /cvsroot/nmh/nmh/mts/smtp/smtp.c,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -b -r1.22 -r1.23
--- mts/smtp/smtp.c     29 Apr 2008 20:59:04 -0000      1.22
+++ mts/smtp/smtp.c     29 Apr 2008 21:05:30 -0000      1.23
@@ -1,7 +1,7 @@
 /*
  * smtp.c -- nmh SMTP interface
  *
- * $Id: smtp.c,v 1.22 2008/04/29 20:59:04 pm215 Exp $
+ * $Id: smtp.c,v 1.23 2008/04/29 21:05:30 pm215 Exp $
  *
  * This code is Copyright (c) 2002, by the authors of nmh.  See the
  * COPYRIGHT file in the root directory of the nmh distribution for
@@ -139,6 +139,7 @@
 static char *EHLOset (char *);
 
 #ifdef MPOP
+static int sm_perror (char *fmt, ...);
 /*
  * smtp.c's own static copy of several nmh library subroutines
  */
@@ -509,7 +510,7 @@
 
 #ifdef MPOP
     if (sm_ispool && !sm_wfp) {
-       strlen (strcpy (sm_reply.text, "unable to create new spool file"));
+       sm_reply.length = strlen (strcpy (sm_reply.text, "unable to create new 
spool file"));
        sm_reply.code = NOTOK;
        return RP_BHST;
     }
@@ -681,7 +682,8 @@
 
        case NOTOK: 
            sm_note.code = sm_reply.code;
-           strncpy (sm_note.text, sm_reply.text, sm_note.length = 
sm_reply.length);/* fall */
+           sm_note.length = sm_reply.length;
+           memcpy (sm_note.text, sm_reply.text, sm_reply.length + 1);/* fall */
        case DONE: 
            if (smtalk (SM_RSET, "RSET") == 250 && type == DONE)
                return RP_OK;
@@ -694,7 +696,8 @@
            }
            if (type == NOTOK) {
                sm_reply.code = sm_note.code;
-               strncpy (sm_reply.text, sm_note.text, sm_reply.length = 
sm_note.length);
+               sm_reply.length = sm_note.length;
+               memcpy (sm_reply.text, sm_note.text, sm_note.length + 1);
            }
            break;
     }
@@ -752,20 +755,7 @@
     gp = NULL;
     k = strlen (file) - sizeof(".bulk");
     if ((fp = fopen (file, "r")) == NULL) {
-       int len;
-
-       snprintf (sm_reply.text, sizeof(sm_reply.text),
-               "unable to read %s: ", file);
-       bp = sm_reply.text;
-       len = strlen (bp);
-       bp += len;
-       if ((s = strerror (errno)))
-           strncpy (bp, s, sizeof(sm_reply.text) - len);
-       else
-           snprintf (bp, sizeof(sm_reply.text) - len, "Error %d", errno);
-       sm_reply.length = strlen (sm_reply.text);
-       sm_reply.code = NOTOK;
-       return RP_BHST;
+       return sm_perror("unable to read %s: ", file);
     }
     if (sm_debug) {
        printf ("reading file %s\n", file);
@@ -826,17 +816,7 @@
        if ((cc = write (fileno (sm_wfp), dp, i)) == NOTOK) {
            int len;
 losing3:
-           strcpy (sm_reply.text, "error writing to server: ",
-               sizeof(sm_reply.text));
-           bp = sm_reply.text;
-           len = strlen (bp);
-           bp += len;
-           if ((s = strerror (errno)))
-               strncpy (bp, s, sizeof(sm_reply.text) - len);
-           else
-               snprintf (bp, sizeof(sm_reply.text) - len,
-                       "unknown error %d", errno);
-           sm_reply.length = strlen (sm_reply.text);
+           sm_perror("error writing to server: ");
            goto losing2;
        }
        else
@@ -981,19 +961,7 @@
        for (dp = cp, i = cc; i > 0; dp += j, i -= j)
            if ((j = fread (cp, sizeof(*cp), i, fp)) == OK) {
                if (ferror (fp)) {
-                   int len;
-
-                   snprintf (sm_reply.text, sizeof(sm_reply.text),
-                       "error reading %s: ", file);
-                   bp = sm_reply.text;
-                   len = strlen (bp);
-                   bp += len;
-                   if ((s = strerror (errno)))
-                       strncpy (bp, s, sizeof(sm_reply.text) - len);
-                   else
-                       snprintf (bp, sizeof(sm_reply.text) - len,
-                               "unknown error %d", errno);
-                   sm_reply.length = strlen (sm_reply.text);
+                   sm_perror("error reading %s: ", file);
                    goto losing2;
                }
                cc = dp - cp;
@@ -1380,6 +1348,36 @@
     return RP_BHST;
 }
 
+#ifdef MPOP
+static int
+sm_perror (char *fmt, ...)
+{
+    /* Fill in sm_reply with a suitable error string based on errno.
+     * This isn't particularly MPOP specific, it just happens that that's
+     * the only code that uses it currently.
+     */
+    char *bp, *s;
+    int len, eno = errno;
+
+    va_list ap;
+    va_start(ap,fmt);
+    vsnprintf (sm_reply.text, sizeof(sm_reply.text), fmt, ap);
+    va_end(ap);
+
+    bp = sm_reply.text;
+    len = strlen(bp);
+    bp += len;
+    if ((s = strerror(eno)))
+       snprintf(bp, sizeof(sm_reply.text) - len, "%s", s);
+    else
+       snprintf(bp, sizeof(sm_reply.text) - len, "unknown error %d", eno);
+    
+    sm_reply.length = strlen (sm_reply.text);
+    sm_reply.code = NOTOK;
+
+    return RP_BHST;
+}
+#endif
 
 static int
 smtalk (int time, char *fmt, ...)
@@ -1412,22 +1410,7 @@
                snprintf (file, sizeof(file), "%s%c.bulk", sm_tmpfil,
                                (char) (sm_ispool + 'a' - 1));
                if (rename (sm_tmpfil, file) == NOTOK) {
-                   int len;
-                   char *bp;
-
-                   snprintf (sm_reply.text, sizeof(sm_reply.text),
-                       "error renaming %s to %s: ", sm_tmpfil, file);
-                   bp = sm_reply.text;
-                   len = strlen (bp);
-                   bp += len;
-                   if ((s = strerror (errno)))
-                       strncpy (bp, s, sizeof(sm_reply.text) - len);
-                   else
-                       snprintf (bp, sizeof(sm_reply.text) - len,
-                               "unknown error %d", errno);
-                   sm_reply.length = strlen (sm_reply.text);
-                   sm_reply.code = NOTOK;
-                   return RP_BHST;
+                   return sm_perror("error renaming %s to %s: ", sm_tmpfil, 
file);
                }
                fclose (sm_wfp);
                if (sm_wfp = fopen (sm_tmpfil, "w"))
@@ -1646,6 +1629,7 @@
            sm_reply.code = code;
            more = cont;
            if (bc <= 0) {
+               /* can never fail to 0-terminate because of size of buffer vs 
fixed string */
                strncpy (buffer, sm_noreply, sizeof(buffer));
                bp = buffer;
                bc = strlen (sm_noreply);
@@ -1653,12 +1637,14 @@
        }
 
        if ((i = min (bc, rc)) > 0) {
-           strncpy (rp, bp, i);
+           memcpy (rp, bp, i);
+           rp += i;
+           rc -= i;
+           i = strlen(sm_moreply);
+           if (more && rc > i + 1) {
+               memcpy (rp, sm_moreply, i); /* safe because of check in if() */
            rp += i;
            rc -= i;
-           if (more && rc > strlen (sm_moreply) + 1) {
-               strncpy (sm_reply.text + rc, sm_moreply, sizeof(sm_reply.text) 
- rc);
-               rc += strlen (sm_moreply);
            }
        }
        if (more)
@@ -1672,6 +1658,7 @@
        }
 
        sm_reply.length = rp - sm_reply.text;
+       sm_reply.text[sm_reply.length] = 0;
        return sm_reply.code;
     }
     return NOTOK;

Index: mts/smtp/smtp.h
===================================================================
RCS file: /cvsroot/nmh/nmh/mts/smtp/smtp.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -b -r1.2 -r1.3
--- mts/smtp/smtp.h     7 Jul 2000 03:48:04 -0000       1.2
+++ mts/smtp/smtp.h     29 Apr 2008 21:05:30 -0000      1.3
@@ -2,7 +2,7 @@
 /*
  * smtp.h -- definitions for the nmh SMTP Interface
  *
- * $Id: smtp.h,v 1.2 2000/07/07 03:48:04 kim Exp $
+ * $Id: smtp.h,v 1.3 2008/04/29 21:05:30 pm215 Exp $
  */
 
 /* various modes for SMTP */
@@ -11,6 +11,9 @@
 #define        S_SOML  2
 #define        S_SAML  3
 
+/* length is the length of the string in text[], which is also NUL
+ * terminated, so s.text[s.length] should always be 0.
+ */
 struct smtp {
     int code;
     int length;