Date: Sun, 28 Jun 1998 03:42:25 -0600
From: "Aaron D. Gifford" <agifford@infowest.com.>
To: security@FreeBSD.ORG
Subject: UIDL overruns in qpopper
When I first saw patches for pop_dropcopy.c that limit the uidl string
length to 128, I couldn't see any overrun potential there. Then Miquel
van Smoorenburg <miquels@CISTRON.NL.> explained that the actual overrun
occur in pop_uidl.c but can be prevented in pop_dropcopy.c. I looked at
pop_uidl.c and sure enough, Miquel was correct. Further investigation
revealed that another potential buffer overrun can occur in some cases
where a huge From: header occurs and the EUIDL command is used by a POP
client.
The patch below constitute several changes I use on my FreeBSD boxen.
It includes a fix for the signal 11 problem that some have reported
after patch-ag is applied. It include a POP parameter limiting patch
which you might want to avoid (it is the patch to the pop_parse.c file)
-- I like it and it has been working on a 5,000-user system with no
known problems yet -- but it might break something I don't know about
or violate POP protocl. It includes a fix to the UIDL overrun by
limiting the size of the UIDL data in pop_dropcopy.c. It also fixes
another potentil UIDL overrun by reducing a buffer in pop_uidl.c.
Finally, it also includes 2 cosmetic fixes that I like because my log
files are more readable -- one change to pop_init.c and another to
pop_auth.c.
Whew!
Please be aware that my mail software might wrap a line or two of the
patch when I send this and cause it to break.
Here goes:
==========
diff -p work/qpopper2.41beta1/pop_auth.c work2/qpopper2.41beta1/pop_auth.c
work/qpopper2.41beta1/pop_auth.c Wed Nov 19 14:20:38 1997
--- work2/qpopper2.41beta1/pop_auth.c Sat Jun 27 23:34:14 1998
*************** int pop_auth (p)
23,29 ****
POP * p;
{
/* Tell the user that this command is not supported */
! return (pop_msg(p,POP_FAILURE,"This command is not supported yet"));
}
--- 23,29 ----
POP * p;
{
/* Tell the user that this command is not supported */
! return (pop_msg(p,POP_FAILURE,"The auth command is not supported yet"));
}
diff -p work/qpopper2.41beta1/pop_dropcopy.c work2/qpopper2.41beta1/pop_dropcopy.c
work/qpopper2.41beta1/pop_dropcopy.c Sun Jun 28 12:58:14 1998
--- work2/qpopper2.41beta1/pop_dropcopy.c Sun Jun 28 13:07:47 1998
*************** POP *p;
489,495 ****
/* Skip over header string */
cp = &buffer[7];
while (*cp && (*cp == ' ' || *cp == '\t')) cp++;
! if(strlen(cp) < DIG_SIZE) /* To account for the empty UIDL string */
{
uidl_found--; /*roll over as though it hasn't seen anything*/
continue;
--- 489,501 ----
/* Skip over header string */
cp = &buffer[7];
while (*cp && (*cp == ' ' || *cp == '\t')) cp++;
! /*
! * The UIDL digest SHOULD be approx. 32 chars long,
! * so reject/skip any X-UIDL: lines that don't fit
! * this profile. A new X-UIDL: line will be created
! * for any messages that don't have a valid one.
! */
! if(strlen(cp) < DIG_SIZE || strlen(cp) > DIG_SIZE * 3)
{
uidl_found--; /*roll over as though it hasn't seen anything*/
continue;
diff -p work/qpopper2.41beta1/pop_init.c work2/qpopper2.41beta1/pop_init.c
work/qpopper2.41beta1/pop_init.c Wed Nov 19 14:20:38 1997
--- work2/qpopper2.41beta1/pop_init.c Sat Jun 27 23:38:54 1998
*************** char ** argmessage;
281,288 ****
ch = gethostbyaddr((char *) &cs.sin_addr, sizeof(cs.sin_addr), AF_INET);
if (ch == NULL){
pop_log(p,POP_PRIORITY,
! "(v%s) Unable to get canonical name of client, err = %d",
! VERSION, errno);
p->client = p->ipaddr;
}
/* Save the cannonical name of the client host in
--- 281,288 ----
ch = gethostbyaddr((char *) &cs.sin_addr, sizeof(cs.sin_addr), AF_INET);
if (ch == NULL){
pop_log(p,POP_PRIORITY,
! "(v%s) Unable to get canonical name of client [%], err = %d",
! VERSION, p->ipaddr, errno);
p->client = p->ipaddr;
}
/* Save the cannonical name of the client host in
diff -p work/qpopper2.41beta1/pop_msg.c work2/qpopper2.41beta1/pop_msg.c
work/qpopper2.41beta1/pop_msg.c Sun Jun 28 12:58:15 1998
--- work2/qpopper2.41beta1/pop_msg.c Sat Jun 27 23:25:59 1998
*************** va_dcl
61,67 ****
/* Point past the POP status indicator in the message message */
l = strlen(mp);
! len -= l, mp += l;
/* Append the message (formatted, if necessary) */
if (format)
--- 61,74 ----
/* Point past the POP status indicator in the message message */
l = strlen(mp);
! mp += l;
! /*
! * Subtract an additional 2 from the remaining buffer length
! * so that after the vsnprintf()/snprintf() calls there will
! * still be enough buffer space to append a "\r\n" even in a
! * worst-case scenario.
! */
! len -= l - 2;
/* Append the message (formatted, if necessary) */
if (format)
*************** va_dcl
90,97 ****
(p->user ? p->user : "(null)"), p->client, message);
/* Append the <CR><LF> */
! len -= strlen(message);
! (void)strncat(message, len, "\r\n");
/* Send the message to the client */
(void)fputs(message,p->output);
--- 97,104 ----
(p->user ? p->user : "(null)"), p->client, message);
/* Append the <CR><LF> */
! len -= strlen(mp);
! (void)strncat(message, "\r\n", len);
/* Send the message to the client */
(void)fputs(message,p->output);
diff -p work/qpopper2.41beta1/pop_parse.c work2/qpopper2.41beta1/pop_parse.c
work/qpopper2.41beta1/pop_parse.c Wed Nov 19 14:20:38 1997
--- work2/qpopper2.41beta1/pop_parse.c Sat Jun 27 22:58:17 1998
*************** char * buf; /* Pointer
26,31 ****
--- 26,32 ----
{
char * mp;
register int i;
+ register int parmlen;
/* Loop through the POP command array */
for (mp = buf, i = 0; ; i++) {
*************** char * buf; /* Pointer
45,52 ****
/* Point to the start of the token */
p->pop_parm[i] = mp;
/* Search for the first space character (end of the token) */
! while (!isspace(*mp) && *mp) mp++;
/* Delimit the token with a null */
if (*mp) *mp++ = 0;
--- 46,75 ----
/* Point to the start of the token */
p->pop_parm[i] = mp;
+ /* Start counting the length of this token */
+ parmlen = 0;
+
/* Search for the first space character (end of the token) */
! while (!isspace(*mp) && *mp) {
! mp++;
! parmlen++;
! if (parmlen > MAXPARMLEN) {
! /* Truncate parameter to the max. allowable size */
! *mp = '\0';
!
! /* Fail with an appropriate message */
! if (i == 0) {
! pop_msg(p,POP_FAILURE,
! "Command \"%s\" (truncated) exceedes maximum permitted size.",
! p->pop_command);
! } else {
! pop_msg(p,POP_FAILURE,
! "Argument %d \"%s\" (truncated) exceeds maximum permitted size.",
! i, p->pop_parm[i]);
! }
! return(-1);
! }
! }
/* Delimit the token with a null */
if (*mp) *mp++ = 0;
diff -p work/qpopper2.41beta1/pop_uidl.c work2/qpopper2.41beta1/pop_uidl.c
work/qpopper2.41beta1/pop_uidl.c Wed Nov 19 14:20:38 1997
--- work2/qpopper2.41beta1/pop_uidl.c Sun Jun 28 13:09:56 1998
*************** from_hdr(p, mp)
fseek(p->drop, mp->offset, 0);
while (fgets(buf, sizeof(buf), p->drop) != NULL) {
--- 101,112 ----
POP *p;
MsgInfoList *mp;
{
! /*
! * Shorten this buffer so that an extra-long From: header
! * won't overflow the buffers in the pop_euidl() where
! * this function is called. 128 should be sufficient.
! */
! static char buf[MAXLINELEN - 128], *cp;
fseek(p->drop, mp->offset, 0);
while (fgets(buf, sizeof(buf), p->drop) != NULL) {
diff -p work/qpopper2.41beta1/popper.h work2/qpopper2.41beta1/popper.h
work/qpopper2.41beta1/popper.h Sun Jun 28 12:58:15 1998
--- work2/qpopper2.41beta1/popper.h Sun Jun 28 11:56:10 1998
***************
#ifndef OSF1
--- 59,65 ----
#define MAXMSGLINELEN MAXLINELEN
#define MAXCMDLEN 4
#define MAXPARMCOUNT 5
! #define MAXPARMLEN 32 /* Large enough for 32-byte APOP parm. */
#define ALLOC_MSGS 20
#ifndef OSF1
Have fun, but not too much! ;)
Aaron out.
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe security" in the body of the message
