[LWN Logo]

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--