Date: Tue, 25 Apr 2000 09:10:54 -0700 From: Claus Assmann <ca+bugtraq@ZARDOC.ENDMAIL.ORG> Subject: Re: unsafe fgets() in sendmail's mail.local To: BUGTRAQ@SECURITYFOCUS.COM --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii On Mon, Apr 24, 2000, 3APA3A wrote: > Topic: > unsafe fgets() in sendmail's mail.local > 1. Possibility to insert LMTP commands into e-mail message > 2. Possibility of deadlock between sendmail and mail.local > 3. Possibility to corrupt user's mailbox > 4. Possibility to change e-mail headers of the message in user's > mailbox > Vulnerable software: > Problems 1 and 2: sendmail before 8.10.0 (8.9.3 tested), all > platforms > Problems 3 and 4: sendmail 8.10.0 and 8.10.1 (8.10.1 tested) > under Solaris only Thanks for the notification and your help to create a patch. The attached patch will be in the next release of sendmail. PS: Content-Length: shouldn't be used anyway :-) --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="p.m.c" Index: mail.local.c =================================================================== RCS file: /cvs/mail.local/mail.local.c,v retrieving revision 8.145 retrieving revision 8.146 diff -u -r8.145 -r8.146 --- mail.local.c 2000/04/25 15:27:08 8.145 +++ mail.local.c 2000/04/25 15:48:32 8.146 @@ -746,7 +746,8 @@ FILE *fp = NULL; time_t tval; bool eline; - bool fullline = TRUE; + bool fullline = TRUE; /* current line is terminated */ + bool prevfl; /* previous line was terminated */ char line[2048]; int fd; char tmpbuf[sizeof _PATH_LOCTMP + 1]; @@ -783,16 +784,19 @@ #endif /* CONTENTLENGTH */ line[0] = '\0'; - for (eline = TRUE; fgets(line, sizeof(line), stdin); ) + eline = TRUE; + while (fgets(line, sizeof(line), stdin) != (char *)NULL) { size_t line_len = 0; int peek; + + prevfl = fullline; /* preserve state of previous line */ while (line[line_len] != '\n' && line_len < sizeof(line) - 2) line_len++; line_len++; /* Check for dot-stuffing */ - if (fullline && lmtprcpts && line[0] == '.') + if (prevfl && lmtprcpts && line[0] == '.') { if (line[1] == '\n' || (line[1] == '\r' && line[2] == '\n')) @@ -801,7 +805,7 @@ line_len--; } - /* Check to see if we have the full line from the fgets() */ + /* Check to see if we have the full line from fgets() */ fullline = FALSE; if (line_len > 0) { @@ -809,12 +813,11 @@ { if (line_len >= 2 && line[line_len - 2] == '\r') - { - (void) strlcpy(line + line_len - 2, - "\n", sizeof line - - line_len + 2); + { + line[line_len - 2] = '\n'; + line[line_len - 1] = '\0'; line_len--; - } + } fullline = TRUE; } else if (line[line_len - 1] == '\r') @@ -834,7 +837,7 @@ fullline = TRUE; #ifdef CONTENTLENGTH - if (line[0] == '\n' && HeaderLength == 0) + if (prevfl && line[0] == '\n' && HeaderLength == 0) { eline = FALSE; HeaderLength = ftell(fp); @@ -849,7 +852,7 @@ } } #else /* CONTENTLENGTH */ - if (line[0] == '\n') + if (prevfl && line[0] == '\n') eline = TRUE; #endif /* CONTENTLENGTH */ else @@ -860,10 +863,17 @@ eline = FALSE; #ifdef CONTENTLENGTH /* discard existing "Content-Length:" headers */ - if (HeaderLength == 0 && + if (prevfl && HeaderLength == 0 && (line[0] == 'C' || line[0] == 'c') && strncasecmp(line, ContentHdr, 15) == 0) - continue; + { + /* + ** be paranoid: clear the line + ** so no "wrong matches" may occur later + */ + line[0] = '\0'; + continue; + } #endif /* CONTENTLENGTH */ } --vkogqOf2sHV7VnPd--