I use NMH 1.0.4_1 on FreeBSD, reading mail using POP3. I am unable to
collect messages containing very long lines (greater than 1K
characters). Such lines get cut up into 1K pieces, each terminated
with a few characters of garbage. "inc -pack foo" appears to work
better, but actually omits every 1024th character of the output.
Sometimes inc crashes. In any case the POP3 session is aborted and so
subsequent messages are not retrieved.
Reproducing this problem is easy (if one has an MTA which allows long
lines in body text). Simply try to 'inc' a message containing a
series of lines of increasing length (1022 characters, 1023
characters, ... 1026 characters). I won't include such lines in this
message, as I assume some readers of this list are using inc with
POP3.
I have a patch for NMH 1.0.4, attached below, which fixes this bug.
Is it possible to get this incorporated into NMH? Are any future
releases of NMH planned? Nothing in the CVS tree seems to have
changed since 2001-03-17. I previously (back in 2002-01) sent an
earlier version of this patch to nmh-bugs. I have received no
acknowledgement or response. Does anyone read this list? Has NMH
been abandoned? Am I the only NMH user left in the world?
I am also in contact with Scott Blachowicz
<Scott(_dot_)Blachowicz(_at_)seaslug(_dot_)org>, the maintainer of the NMH port
for
FreeBSD, and he recommended I try nmh-bugs again.
The cause of the bug is a number of buffer overflow bugs in the POP
handling (multiline/getline and its callers). The effect is to
corrupt message contents or to crash whichever NMH component is
running POP.
The patch changes the popsbr.c getline() and multiline() functions so
that they handle long lines better. Short lines are now terminated
with \n\0; parts of long lines are terminated with \0. getline() and
multiline() now return the number of characters copied.
I have had to go through popsbr.c and various other files which use
these functions (or other popsbr.c functions) in order to make them
all understand the new interface. I hope I have got everything right,
but I can't be sure as there are large parts of NMH which I don't use.
Certainly inc now works OK for me.
Nick Barnes
Director
Ravenbrook Limited
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/h/popsbr.h ./h/popsbr.h
--- /usr/ports/mail/nmh/work/nmh-1.0.4/h/popsbr.h Fri Feb 4 01:32:12 2000
+++ ./h/popsbr.h Thu Jan 31 11:08:28 2002
@@ -25,11 +25,11 @@
int pop_init (char *, char *, char *, int, int, int);
int pop_fd (char *, int, char *, int);
int pop_stat (int *, int *);
-int pop_retr (int, int (*)());
+int pop_retr (int, int (*)(const char *, int));
int pop_dele (int);
int pop_noop (void);
int pop_rset (void);
-int pop_top (int, int, int (*)());
+int pop_top (int, int, int (*)(const char *, int));
int pop_quit (void);
int pop_done (void);
@@ -40,7 +40,7 @@
#endif
#ifdef BPOP
-int pop_xtnd (int (*)(), char *, ...);
+int pop_xtnd (int (*)(const char *, int), char *, ...);
#endif
#if defined(MPOP) && !defined(NNTP)
@@ -50,7 +50,7 @@
#if !defined(NNTP) && defined(MPOP)
/* otherwise they are static functions */
int command(const char *, ...);
-int multiline(void);
+int multiline(int);
#endif
/*
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/inc.c ./uip/inc.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/inc.c Fri Mar 17 20:11:04 2000
+++ ./uip/inc.c Thu Jan 31 14:00:34 2002
@@ -153,8 +153,8 @@
#ifdef POP
int done(int);
-static int pop_action(char *);
-static int pop_pack(char *);
+static int pop_action(const char *, int);
+static int pop_pack(const char *, int);
static int map_count(void);
#endif
@@ -909,26 +909,26 @@
}
static int
-pop_action (char *s)
+pop_action (const char *s, int l)
{
- fprintf (pf, "%s\n", s);
- stop += strlen (s) + 1;
+ fprintf (pf, "%s", s);
+ stop += strlen (s);
return 0; /* Is return value used? This was missing before 1999-07-15. */
}
static int
-pop_pack (char *s)
+pop_pack (const char *s, int l)
{
int j;
char buffer[BUFSIZ];
- snprintf (buffer, sizeof(buffer), "%s\n", s);
+ strncpy (buffer, sizeof(buffer), s);
for (j = 0; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++)
continue;
for (j = 0; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++)
continue;
fputs (buffer, pf);
- size += strlen (buffer) + 1;
+ size += strlen (buffer);
return 0; /* Is return value used? This was missing before 1999-07-15. */
}
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/msh.c ./uip/msh.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/msh.c Mon Mar 6 20:19:04 2000
+++ ./uip/msh.c Thu Jan 31 11:15:35 2002
@@ -192,10 +192,10 @@
#ifdef BPOP
# ifdef NNTP
-static int pop_statmsg (char *);
+static int pop_statmsg (const char *, int);
# endif /* NNTP */
static int read_pop (void);
-static int pop_action (char *);
+static int pop_action (const char *, int);
#endif /* BPOP */
static void m_gMsgs (int);
@@ -884,7 +884,7 @@
static int pop_base = 0;
static int
-pop_statmsg (char *s)
+pop_statmsg (const char *s, int l /* line length (unused) */)
{
register int i, n;
@@ -913,9 +913,9 @@
static int
-pop_action (char *s)
+pop_action (char *s, int linelen)
{
- fprintf (yp, "%s\n", s);
+ fprintf (yp, "%s", s);
}
#endif /* BPOP */
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/mshcmds.c ./uip/mshcmds.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/mshcmds.c Fri Feb 4 20:28:24 2000
+++ ./uip/mshcmds.c Thu Jan 31 13:34:00 2002
@@ -2194,15 +2194,14 @@
for (;;) {
int size;
- switch (pop_multiline ()) {
+ switch (pop_multiline (0)) {
case NOTOK:
fprintf (stderr, "%s", response);
- /* and fall... */
- case DONE:
+ break;
+ case 0:
fprintf (stderr,"\n");
break;
-
- case OK:
+ default:
if (sscanf (response, "%d %d", &msgnum, &size) == 2
&& mp->lowmsg <= msgnum
&& msgnum <= mp->hghmsg
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popi.c ./uip/popi.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popi.c Fri Feb 4 20:28:24 2000
+++ ./uip/popi.c Thu Jan 31 14:06:01 2002
@@ -84,6 +84,9 @@
*/
int sc_width (void); /* from termsbr.c */
+static int
+retr_action (const char *rsp, int linelen);
+static int retr_action_flag;
int
main (int argc, char **argv)
@@ -347,7 +350,7 @@
if (cp)
*cp = ' ';
pop_command ("%s%s", popicmds[i].sw, cp ? cp : "");
- printf ("%s\n", response);
+ printf ("%s", response);
break;
case LISTCMD:
@@ -355,19 +358,18 @@
*cp = ' ';
if (pop_command ("%s%s", popicmds[i].sw, cp ? cp : "")
== OK) {
- printf ("%s\n", response);
+ printf ("%s", response);
if (!cp)
for (;;) {
- switch (pop_multiline ()) {
- case DONE:
- strcpy (response, ".");
- /* and fall... */
+ switch (pop_multiline (0)) {
+ case 0:
+ printf(".\n");
+ break;
case NOTOK:
- printf ("%s\n", response);
+ printf ("%s", response);
break;
-
- case OK:
- printf ("%s\n", response);
+ default:
+ printf ("%s", response);
continue;
}
break;
@@ -380,10 +382,12 @@
advise (NULL, "missing argument to %s", buffer);
break;
}
- retr_action (NULL, OK);
+ retr_action_flag = OK;
+ retr_action (NULL, 0);
pop_retr (atoi (++cp), retr_action);
- retr_action (NULL, DONE);
- printf ("%s\n", response);
+ retr_action_flag = DONE;
+ retr_action (NULL, 0);
+ printf ("%s", response);
break;
case SCANCMD:
@@ -413,7 +417,7 @@
*fp = '\0';
pop_command ("xtnd scan %d \"%s\"", width, ep);
- printf ("%s\n", response);
+ printf ("%s", response);
free (ep);
}
@@ -430,7 +434,7 @@
static int
-retr_action (char *rsp, int flag)
+retr_action (const char *rsp, int len)
{
static FILE *fp;
@@ -438,7 +442,7 @@
static int msgnum;
static char *cp;
- if (flag == OK) {
+ if (retr_action_flag == OK) {
if (!(mp = folder_realloc (mp, mp->lowoff, msgnum = mp->hghmsg +
1)))
adios (NULL, "unable to allocate folder storage");
@@ -471,7 +475,7 @@
return;
}
- fprintf (fp, "%s\n", rsp);
+ fprintf (fp, "%s", rsp);
}
diff -bru /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popsbr.c ./uip/popsbr.c
--- /usr/ports/mail/nmh/work/nmh-1.0.4/uip/popsbr.c Fri Feb 4 01:32:12 2000
+++ ./uip/popsbr.c Thu Jan 31 14:19:06 2002
@@ -63,10 +63,10 @@
#if defined(NNTP) || !defined(MPOP)
/* otherwise they are not static functions */
static int command(const char *, ...);
-static int multiline(void);
+static int multiline(int more);
#endif
-static int traverse (int (*)(), const char *, ...);
+static int traverse (int (*)(const char *, int), const char *, ...);
static int vcommand(const char *, va_list);
static int getline (char *, int, FILE *);
static int putline (char *, FILE *);
@@ -122,6 +122,7 @@
{
int fd1, fd2;
char buffer[BUFSIZ];
+ int linelen;
#ifdef APOP
int apop;
@@ -170,10 +171,10 @@
SIGNAL (SIGPIPE, SIG_IGN);
- switch (getline (response, sizeof response, input)) {
- case OK:
+ linelen = getline (response, sizeof response, input);
+ if (linelen > 0) { /* some bytes */
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", response);
#ifndef NNTP
if (*response == '+') {
# ifndef KPOP
@@ -183,8 +184,7 @@
if (cp && command ("APOP %s", cp) != NOTOK)
return OK;
- }
- else
+ } else
# endif /* APOP */
if (command ("USER %s", user) != NOTOK
&& command ("%s %s", rpop ? "RPOP" : (pophack++, "PASS"),
@@ -206,17 +206,13 @@
command ("QUIT");
strncpy (response, buffer, sizeof(response));
/* and fall */
+ }
- case NOTOK:
- case DONE:
if (poprint)
fprintf (stderr, "%s\n", response);
fclose (input);
fclose (output);
return NOTOK;
- }
-
- return NOTOK; /* NOTREACHED */
}
#ifdef NNTP
@@ -303,7 +299,7 @@
#ifdef NNTP
int
-pop_exists (int (*action)())
+pop_exists (int (*action)(const char *, int))
{
#ifdef XMSGS /* hacked into NNTP 1.5 */
if (traverse (action, "XMSGS %d-%d", (targ_t) xtnd_first, (targ_t)
xtnd_last) == OK)
@@ -326,6 +322,7 @@
#endif
{
int i;
+ int linelen;
#ifndef BPOP
int *ids = NULL;
#endif
@@ -356,14 +353,15 @@
if (command ("LIST") == NOTOK)
return NOTOK;
- for (i = 0; i < *nmsgs; i++)
- switch (multiline ()) {
- case NOTOK:
+ for (i = 0; i < *nmsgs; i++) {
+ /* LIST lines must be smaller than our buffer */
+ switch(multiline (0)) {
+ case NOTOK: /* EOF or I/O error */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
*nmsgs = ++i;
return OK;
- case OK:
+ default:
*msgs = *bytes = 0;
if (ids) {
*ids = 0;
@@ -374,13 +372,14 @@
sscanf (response, "%d %d", msgs++, bytes++);
break;
}
+ }
for (;;)
- switch (multiline ()) {
- case NOTOK:
+ switch (multiline (0)) {
+ case NOTOK: /* EOF or I/O error */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
return OK;
- case OK:
+ default: /* a multi-line line */
break;
}
#else /* NNTP */
@@ -390,7 +389,7 @@
int
-pop_retr (int msgno, int (*action)())
+pop_retr (int msgno, int (*action)(const char *, int))
{
#ifndef NNTP
return traverse (action, "RETR %d", (targ_t) msgno);
@@ -401,11 +400,13 @@
static int
-traverse (int (*action)(), const char *fmt, ...)
+traverse (int (*action)(const char *, int), const char *fmt, ...)
{
int result;
va_list ap;
char buffer[sizeof(response)];
+ int linelen;
+ int more = 0;
va_start(ap, fmt);
result = vcommand (fmt, ap);
@@ -415,19 +416,23 @@
return NOTOK;
strncpy (buffer, response, sizeof(buffer));
- for (;;)
- switch (multiline ()) {
- case NOTOK:
+ for (;;) {
+ linelen = multiline(more);
+ switch(linelen) {
+ case NOTOK: /* EOF or I/O error. */
return NOTOK;
- case DONE:
+ case 0: /* end of multi-line */
strncpy (response, buffer, sizeof(response));
return OK;
- case OK:
- (*action) (response);
+ default: /* some bytes */
+ /* does this include the end of a line? */
+ more = (response[linelen-1] != '\n');
+ (*action) (response, linelen);
break;
}
+ }
}
@@ -462,7 +467,7 @@
int
-pop_top (int msgno, int lines, int (*action)())
+pop_top (int msgno, int lines, int (*action)(const char *, int))
{
#ifndef NNTP
return traverse (action, "TOP %d %d", (targ_t) msgno, (targ_t) lines);
@@ -474,7 +479,7 @@
#ifdef BPOP
int
-pop_xtnd (int (*action)(), char *fmt, ...)
+pop_xtnd (int (*action)(const char *, int), char *fmt, ...)
{
int result;
va_list ap;
@@ -521,7 +526,7 @@
xtnd_first = atoi (ap[2]);
xtnd_last = atoi (ap[3]);
- (*action) (buffer);
+ (*action) (buffer, strlen(buffer));
return OK;
} else { /* XTND "BBOARDS" */
@@ -577,6 +582,7 @@
vcommand (const char *fmt, va_list ap)
{
char *cp, buffer[BUFSIZ];
+ int linelen;
vsnprintf (buffer, sizeof(buffer), fmt, ap);
if (poprint) {
@@ -595,81 +601,113 @@
if (putline (buffer, output) == NOTOK)
return NOTOK;
- switch (getline (response, sizeof response, input)) {
- case OK:
+ linelen = getline (response, sizeof response, input);
+ if (linelen > 0) {
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", response);
#ifndef NNTP
return (*response == '+' ? OK : NOTOK);
#else /* NNTP */
return (*response < CHAR_ERR ? OK : NOTOK);
#endif /* NNTP */
-
- case NOTOK:
- case DONE:
+ }
if (poprint)
- fprintf (stderr, "%s\n", response);
+ fprintf (stderr, "%s", response);
return NOTOK;
- }
-
- return NOTOK; /* NOTREACHED */
}
+/* multiline(more)
+ *
+ * Handle multi-line responses (RFC1939, section 3). A line which is
+ * a lone '.' ends the response. Any other line which begins with '.'
+ * is "byte-stuffed" by prepending another '.'.
+ *
+ * If the 'more' argument is non-zero, we are getting more of a
+ * part-read line, so the '.' handling does not apply.
+ *
+ * On I/O error or EOF, return NOTOK.
+ * If we have reached the end of the response, return 0.
+ * Otherwise, return the number of bytes.
+ */
+
#if defined(MPOP) && !defined(NNTP)
int
-multiline (void)
+multiline (int more)
#else
static int
-multiline (void)
+multiline (int more)
#endif
{
- char buffer[BUFSIZ + TRMLEN];
-
- if (getline (buffer, sizeof buffer, input) != OK)
+ char buffer[BUFSIZ];
+ int linelen = getline (buffer, sizeof buffer, input);
+ if (linelen <= 0)
return NOTOK;
#ifdef DEBUG
if (poprint)
- fprintf (stderr, "<--- %s\n", response);
+ fprintf (stderr, "<--- %s", buffer);
#endif DEBUG
- if (strncmp (buffer, TRM, TRMLEN) == 0) {
- if (buffer[TRMLEN] == 0)
- return DONE;
- else
- strncpy (response, buffer + TRMLEN, sizeof(response));
+ if ((!more) &&
+ (strncmp (buffer, TRM, TRMLEN) == 0)) { /* leading '.' */
+ if (buffer[TRMLEN] == '\n') { /* lone '.': end of multi-line */
+ return 0;
+ } else {
+ strncpy (response, buffer+TRMLEN, sizeof(response));
+ return (linelen - TRMLEN);
+ }
}
- else
strncpy (response, buffer, sizeof(response));
-
- return OK;
+ return linelen;
}
+/* getline(buf, bufsize, file)
+ * get a CRLF terminated line, and return it with \n in place of CRLF.
+ * If we don't reach a terminating CRLF, this will return as much as
+ * it can. NUL-terminate the result.
+ *
+ * Return value is the number of data bytes, including any \n, not
+ * including the NUL.
+ *
+ * Return NOTOK if we get a connection error.
+ * Return 0 if there are no bytes (EOF).
+ *
+ * SMTP says that all lines are terminated with CRLF and CR and LF do
+ * not appear bare. So we can just discard all CRs. This avoids the problem
+ * of a CRLF being split across buffers. */
static int
getline (char *s, int n, FILE *iop)
{
int c;
char *p;
+ char *limit = s+n-1;
p = s;
- while (--n > 0 && (c = fgetc (iop)) != EOF)
- if ((*p++ = c) == '\n')
+ for (;;) {
+ if (p == limit)
+ break;
+ while ((c = fgetc(iop)) == '\r') /* drop CRs */
+ ;
+ if (c == EOF)
break;
+ *p++ = c;
+ if (c == '\n')
+ break;
+ }
+
if (ferror (iop) && c != EOF) {
- strncpy (response, "error on connection", sizeof(response));
+ strncpy (response, "error on connection\n", sizeof(response));
return NOTOK;
}
if (c == EOF && p == s) {
- strncpy (response, "connection closed by foreign host",
sizeof(response));
- return DONE;
+ strncpy (response,
+ "connection closed by foreign host\n", sizeof(response));
}
- *p = 0;
- if (*--p == '\n')
- *p = 0;
- if (*--p == '\r')
+
+ /* add terminating NUL */
*p = 0;
- return OK;
+ return p-s;
}