bugs in spool_copy_message()

[Philip Chambers]

On Wed, 27 Oct 2004 11:04:33 -0700 (PDT) Andrew Morgan <morgan at orst.edu> wrote:

> 
> 
> On Wed, 27 Oct 2004, Philip Chambers wrote:
> 
> > I have just found two flaws in the code which takes a message into cyrus (typically
> > during the DATA phase of LMTP.  I am amazed that one has existed for so long.
> >
> > It means that messages with a line longer that 8190 bytes will be rejected with the
> > error "Message contains NUL characters".  (Confirmed in testing.)
> 
> I've had some of our users report the "Message contains NUL characters"
> bounce message, but I could never figure out why.  If you come up with a
> patch for this, I'd be very interested in applying it here.  Note: We are
> still running cyrus-imapd 2.1.16 here...
> 
> 	Andy

In 2.1.16 this code may be in copy_message() in lmtpengine.c, you will need to check.

I now have a fix which I believe good and has worked under test. It seems to me that 
the original code is too complex and can be simplified substantially.  To that end I 
have made a small change to prot_fgets() in lib/prot.c: all calls to prot_fgets() 
just check for a zero/non-zero result, so I have changed it to return "p" rather 
than "buf".  That means that spool_copy_message() can know exactly where the end of 
the data is that has been read.  The code in spool_copy_message() becomes much 
simpler:

    ....
    char *end;

    /* -2: Might need room to add a \r\n\0 set */
    while (end = prot_fgets(buf, sizeof(buf)-2, fin)) {
	p = buf + strlen(buf) - 1;
        if (p < end-2) {
	    /* strlen stopped before end */
	    r = IMAP_MESSAGE_CONTAINSNULL;
	    continue; /* need to eat the rest of the message */
	}
	else if (p[0] == '\r') {
	    /*
	     * We were unlucky enough to get a CR just before we ran
	     * out of buffer--put it back.
	     */
	    prot_ungetc('\r', fin);
	    *p = '\0';
	}
	else if (p[0] == '\n' && (p == buf || p[-1] != '\r')) {
	    /* found an \n without a \r */
	    p[0] = '\r';
	    p[1] = '\n';
	    p[2] = '\0';
	}

	/* Remove any lone CR characters */
        ... as before

Phil.
---------------------------------------
Phil Chambers (postmaster at exeter.ac.uk)
University of Exeter

---
Cyrus Home Page: http://asg.web.cmu.edu/cyrus
Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html