FreeBSD-SA-03:04.sendmail: sendmail header parsing buffer overflow

Approved by:    security-officer (nectar)

7 files changed, 138 insertions, 89 deletions

diff --git a/UPDATING b/UPDATING
index fb5d9dc091e4..bd393586acfb 100644
--- a/UPDATING
+++ b/UPDATING
@@ -16,6 +16,9 @@ minimal number of processes, if possible, for that patch.  For those
 updates that don't have an advisory, or to be safe, you can do a full
 build and install as described in the COMMON ITEMS section.
 
+20030303:	p31	FreeBSD-SA-03:04.sendmail
+	sendmail header parsing buffer overflow, ident parsing bug.
+
 20021026:	p30
 	smrsh bypass bug.
 
diff --git a/contrib/sendmail/src/daemon.c b/contrib/sendmail/src/daemon.c
index f4b07d420871..d02030bfb3c0 100644
--- a/contrib/sendmail/src/daemon.c
+++ b/contrib/sendmail/src/daemon.c
@@ -2776,7 +2776,7 @@ getauthinfo(fd, may_be_forged)
 	if (i < 0 || p == &ibuf[0])
 		goto noident;
 
-	if (*--p == '\n' && *--p == '\r')
+	if (p >= &ibuf[2] && *--p == '\n' && *--p == '\r')
 		p--;
 	*++p = '\0';
 
diff --git a/contrib/sendmail/src/headers.c b/contrib/sendmail/src/headers.c
index a39caeacecfb..2b44e6dbfed7 100644
--- a/contrib/sendmail/src/headers.c
+++ b/contrib/sendmail/src/headers.c
@@ -650,7 +650,8 @@ eatheader(e, full)
 			if (buf[0] != '\0')
 			{
 				if (bitset(H_FROM, h->h_flags))
-					expand(crackaddr(buf), buf, sizeof buf, e);
+					expand(crackaddr(buf, e),
+					       buf, sizeof buf, e);
 				h->h_value = newstr(buf);
 				h->h_flags &= ~H_DEFAULT;
 			}
@@ -973,7 +974,11 @@ priencode(p)
 **	it and replaces it with "$g".  The parse is totally ad hoc
 **	and isn't even guaranteed to leave something syntactically
 **	identical to what it started with.  However, it does leave
-**	something semantically identical.
+**	something semantically identical if possible, else at least
+**	syntactically correct.
+**
+**	For example, it changes "Real Name <real@example.com> (Comment)"
+**	to "Real Name <$g> (Comment)".
 **
 **	This algorithm has been cleaned up to handle a wider range
 **	of cases -- notably quoted and backslash escaped strings.
@@ -982,6 +987,7 @@ priencode(p)
 **
 **	Parameters:
 **		addr -- the address to be cracked.
+**		e -- the current envelope.
 **
 **	Returns:
 **		a pointer to the new version.
@@ -994,28 +1000,50 @@ priencode(p)
 **		be copied if it is to be reused.
 */
 
+#define SM_HAVE_ROOM		((bp < buflim) && (buflim <= bufend))
+
+/*
+**  Append a character to bp if we have room.
+**  If not, punt and return $g.
+*/
+
+#define SM_APPEND_CHAR(c)					\
+	do							\
+	{							\
+		if (SM_HAVE_ROOM)				\
+			*bp++ = (c);				\
+		else						\
+			goto returng;				\
+	} while (0)
+
+#if MAXNAME < 10
+ERROR MAXNAME must be at least 10
+#endif /* MAXNAME < 10 */
+
 char *
-crackaddr(addr)
+crackaddr(addr, e)
 	register char *addr;
+	ENVELOPE *e;
 {
 	register char *p;
 	register char c;
-	int cmtlev;
-	int realcmtlev;
-	int anglelev, realanglelev;
-	int copylev;
-	int bracklev;
-	bool qmode;
-	bool realqmode;
-	bool skipping;
-	bool putgmac = FALSE;
-	bool quoteit = FALSE;
-	bool gotangle = FALSE;
-	bool gotcolon = FALSE;
+	int cmtlev;			/* comment level in input string */
+	int realcmtlev;			/* comment level in output string */
+	int anglelev;			/* angle level in input string */
+	int copylev;			/* 0 == in address, >0 copying */
+	int bracklev;			/* bracket level for IPv6 addr check */
+	bool addangle;			/* put closing angle in output */
+	bool qmode;			/* quoting in original string? */
+	bool realqmode;			/* quoting in output string? */
+	bool putgmac = FALSE;		/* already wrote $g */
+	bool quoteit = FALSE;		/* need to quote next character */
+	bool gotangle = FALSE;		/* found first '<' */
+	bool gotcolon = FALSE;		/* found a ':' */
 	register char *bp;
 	char *buflim;
 	char *bufhead;
 	char *addrhead;
+	char *bufend;
 	static char buf[MAXNAME + 1];
 
 	if (tTd(33, 1))
@@ -1030,25 +1058,22 @@ crackaddr(addr)
 	**  adjusted later if we find them.
 	*/
 
+	buflim = bufend = &buf[sizeof(buf) - 1];
 	bp = bufhead = buf;
-	buflim = &buf[sizeof buf - 7];
 	p = addrhead = addr;
-	copylev = anglelev = realanglelev = cmtlev = realcmtlev = 0;
+	copylev = anglelev = cmtlev = realcmtlev = 0;
 	bracklev = 0;
-	qmode = realqmode = FALSE;
+	qmode = realqmode = addangle = FALSE;
 
 	while ((c = *p++) != '\0')
 	{
 		/*
-		**  If the buffer is overful, go into a special "skipping"
-		**  mode that tries to keep legal syntax but doesn't actually
-		**  output things.
+		**  Try to keep legal syntax using spare buffer space
+		**  (maintained by buflim).
 		*/
 
-		skipping = bp >= buflim;
-
-		if (copylev > 0 && !skipping)
-			*bp++ = c;
+		if (copylev > 0)
+			SM_APPEND_CHAR(c);
 
 		/* check for backslash escapes */
 		if (c == '\\')
@@ -1063,8 +1088,8 @@ crackaddr(addr)
 				p--;
 				goto putg;
 			}
-			if (copylev > 0 && !skipping)
-				*bp++ = c;
+			if (copylev > 0)
+				SM_APPEND_CHAR(c);
 			goto putg;
 		}
 
@@ -1072,8 +1097,14 @@ crackaddr(addr)
 		if (c == '"' && cmtlev <= 0)
 		{
 			qmode = !qmode;
-			if (copylev > 0 && !skipping)
+			if (copylev > 0 && SM_HAVE_ROOM)
+			{
+				if (realqmode)
+					buflim--;
+				else
+					buflim++;
 				realqmode = !realqmode;
+			}
 			continue;
 		}
 		if (qmode)
@@ -1085,15 +1116,15 @@ crackaddr(addr)
 			cmtlev++;
 
 			/* allow space for closing paren */
-			if (!skipping)
+			if (SM_HAVE_ROOM)
 			{
 				buflim--;
 				realcmtlev++;
 				if (copylev++ <= 0)
 				{
 					if (bp != bufhead)
-						*bp++ = ' ';
-					*bp++ = c;
+						SM_APPEND_CHAR(' ');
+					SM_APPEND_CHAR(c);
 				}
 			}
 		}
@@ -1103,7 +1134,7 @@ crackaddr(addr)
 			{
 				cmtlev--;
 				copylev--;
-				if (!skipping)
+				if (SM_HAVE_ROOM)
 				{
 					realcmtlev--;
 					buflim++;
@@ -1114,7 +1145,7 @@ crackaddr(addr)
 		else if (c == ')')
 		{
 			/* syntax error: unmatched ) */
-			if (copylev > 0 && !skipping)
+			if (copylev > 0 && SM_HAVE_ROOM)
 				bp--;
 		}
 
@@ -1132,7 +1163,7 @@ crackaddr(addr)
 
 			/*
 			**  Check for DECnet phase IV ``::'' (host::user)
-			**  or **  DECnet phase V ``:.'' syntaxes.  The latter
+			**  or DECnet phase V ``:.'' syntaxes.  The latter
 			**  covers ``user@DEC:.tay.myhost'' and
 			**  ``DEC:.tay.myhost::user'' syntaxes (bletch).
 			*/
@@ -1141,10 +1172,10 @@ crackaddr(addr)
 			{
 				if (cmtlev <= 0 && !qmode)
 					quoteit = TRUE;
-				if (copylev > 0 && !skipping)
+				if (copylev > 0)
 				{
-					*bp++ = c;
-					*bp++ = *p;
+					SM_APPEND_CHAR(c);
+					SM_APPEND_CHAR(*p);
 				}
 				p++;
 				goto putg;
@@ -1155,41 +1186,43 @@ crackaddr(addr)
 			bp = bufhead;
 			if (quoteit)
 			{
-				*bp++ = '"';
+				SM_APPEND_CHAR('"');
 
 				/* back up over the ':' and any spaces */
 				--p;
-				while (isascii(*--p) && isspace(*p))
+				while (p > addr &&
+				       isascii(*--p) && isspace(*p))
 					continue;
 				p++;
 			}
 			for (q = addrhead; q < p; )
 			{
 				c = *q++;
-				if (bp < buflim)
+				if (quoteit && c == '"')
 				{
-					if (quoteit && c == '"')
-						*bp++ = '\\';
-					*bp++ = c;
+					SM_APPEND_CHAR('\\');
+					SM_APPEND_CHAR(c);
 				}
+				else
+					SM_APPEND_CHAR(c);
 			}
 			if (quoteit)
 			{
 				if (bp == &bufhead[1])
 					bp--;
 				else
-					*bp++ = '"';
+					SM_APPEND_CHAR('"');
 				while ((c = *p++) != ':')
-				{
-					if (bp < buflim)
-						*bp++ = c;
-				}
-				*bp++ = c;
+					SM_APPEND_CHAR(c);
+				SM_APPEND_CHAR(c);
 			}
 
 			/* any trailing white space is part of group: */
-			while (isascii(*p) && isspace(*p) && bp < buflim)
-				*bp++ = *p++;
+			while (isascii(*p) && isspace(*p))
+			{
+				SM_APPEND_CHAR(*p);
+				p++;
+			}
 			copylev = 0;
 			putgmac = quoteit = FALSE;
 			bufhead = bp;
@@ -1198,10 +1231,7 @@ crackaddr(addr)
 		}
 
 		if (c == ';' && copylev <= 0 && !ColonOkInAddr)
-		{
-			if (bp < buflim)
-				*bp++ = c;
-		}
+			SM_APPEND_CHAR(c);
 
 		/* check for characters that may have to be quoted */
 		if (strchr(MustQuoteChars, c) != NULL)
@@ -1229,42 +1259,45 @@ crackaddr(addr)
 
 			/* oops -- have to change our mind */
 			anglelev = 1;
-			if (!skipping)
-				realanglelev = 1;
+			if (SM_HAVE_ROOM)
+			{
+				if (!addangle)
+					buflim--;
+				addangle = TRUE;
+			}
 
 			bp = bufhead;
 			if (quoteit)
 			{
-				*bp++ = '"';
+				SM_APPEND_CHAR('"');
 
 				/* back up over the '<' and any spaces */
 				--p;
-				while (isascii(*--p) && isspace(*p))
+				while (p > addr &&
+				       isascii(*--p) && isspace(*p))
 					continue;
 				p++;
 			}
 			for (q = addrhead; q < p; )
 			{
 				c = *q++;
-				if (bp < buflim)
+				if (quoteit && c == '"')
 				{
-					if (quoteit && c == '"')
-						*bp++ = '\\';
-					*bp++ = c;
+					SM_APPEND_CHAR('\\');
+					SM_APPEND_CHAR(c);
 				}
+				else
+					SM_APPEND_CHAR(c);
 			}
 			if (quoteit)
 			{
 				if (bp == &buf[1])
 					bp--;
 				else
-					*bp++ = '"';
+					SM_APPEND_CHAR('"');
 				while ((c = *p++) != '<')
-				{
-					if (bp < buflim)
-						*bp++ = c;
-				}
-				*bp++ = c;
+					SM_APPEND_CHAR(c);
+				SM_APPEND_CHAR(c);
 			}
 			copylev = 0;
 			putgmac = quoteit = FALSE;
@@ -1276,13 +1309,14 @@ crackaddr(addr)
 			if (anglelev > 0)
 			{
 				anglelev--;
-				if (!skipping)
+				if (SM_HAVE_ROOM)
 				{
-					realanglelev--;
-					buflim++;
+					if (addangle)
+						buflim++;
+					addangle = FALSE;
 				}
 			}
-			else if (!skipping)
+			else if (SM_HAVE_ROOM)
 			{
 				/* syntax error: unmatched > */
 				if (copylev > 0)
@@ -1291,7 +1325,7 @@ crackaddr(addr)
 				continue;
 			}
 			if (copylev++ <= 0)
-				*bp++ = c;
+				SM_APPEND_CHAR(c);
 			continue;
 		}
 
@@ -1299,30 +1333,42 @@ crackaddr(addr)
 	putg:
 		if (copylev <= 0 && !putgmac)
 		{
-			if (bp > bufhead && bp[-1] == ')')
-				*bp++ = ' ';
-			*bp++ = MACROEXPAND;
-			*bp++ = 'g';
+			if (bp > buf && bp[-1] == ')')
+				SM_APPEND_CHAR(' ');
+			SM_APPEND_CHAR(MACROEXPAND);
+			SM_APPEND_CHAR('g');
 			putgmac = TRUE;
 		}
 	}
 
 	/* repair any syntactic damage */
-	if (realqmode)
+	if (realqmode && bp < bufend)
 		*bp++ = '"';
-	while (realcmtlev-- > 0)
+	while (realcmtlev-- > 0 && bp < bufend)
 		*bp++ = ')';
-	while (realanglelev-- > 0)
+	if (addangle && bp < bufend)
 		*bp++ = '>';
-	*bp++ = '\0';
-
+	*bp = '\0';
+	if (bp < bufend)
+		goto success;
+
+ returng:
+	/* String too long, punt */
+	buf[0] = '<';
+	buf[1] = MACROEXPAND;
+	buf[2]= 'g';
+	buf[3] = '>';
+	buf[4]= '\0';
+	sm_syslog(LOG_ALERT, e->e_id,
+		  "Dropped invalid comments from header address");
+
+ success:
 	if (tTd(33, 1))
 	{
 		dprintf("crackaddr=>`");
 		xputs(buf);
 		dprintf("'\n");
 	}
-
 	return buf;
 }
 /*
diff --git a/contrib/sendmail/src/main.c b/contrib/sendmail/src/main.c
index bf976e0f7d52..0593d59421ca 100644
--- a/contrib/sendmail/src/main.c
+++ b/contrib/sendmail/src/main.c
@@ -3133,7 +3133,7 @@ testmodeline(line, e)
 				printf("Usage: /parse address\n");
 				return;
 			}
-			q = crackaddr(p);
+			q = crackaddr(p, e);
 			printf("Cracked address = ");
 			xputs(q);
 			printf("\nParsing %s %s address\n",
diff --git a/contrib/sendmail/src/parseaddr.c b/contrib/sendmail/src/parseaddr.c
index 0a2d8ad8dac2..7bf71dfc3b48 100644
--- a/contrib/sendmail/src/parseaddr.c
+++ b/contrib/sendmail/src/parseaddr.c
@@ -2289,7 +2289,7 @@ remotename(name, m, flags, pstat, e)
 	if (bitset(RF_CANONICAL, flags) || bitnset(M_NOCOMMENT, m->m_flags))
 		fancy = "\201g";
 	else
-		fancy = crackaddr(name);
+		fancy = crackaddr(name, e);
 
 	/*
 	**  Turn the name into canonical form.
diff --git a/contrib/sendmail/src/sendmail.h b/contrib/sendmail/src/sendmail.h
index ffb0b33f56b9..22b6ec7c9797 100644
--- a/contrib/sendmail/src/sendmail.h
+++ b/contrib/sendmail/src/sendmail.h
@@ -270,7 +270,7 @@ extern ADDRESS	NullAddress;	/* a null (template) address [main.c] */
 
 /* functions */
 extern void	cataddr __P((char **, char **, char *, int, int));
-extern char	*crackaddr __P((char *));
+extern char	*crackaddr __P((char *, ENVELOPE *));
 extern bool	emptyaddr __P((ADDRESS *));
 extern ADDRESS	*getctladdr __P((ADDRESS *));
 extern int	include __P((char *, bool, ADDRESS *, ADDRESS **, int, ENVELOPE *));
diff --git a/sys/conf/newvers.sh b/sys/conf/newvers.sh
index a7658d5cea83..b60ca71900e6 100644
--- a/sys/conf/newvers.sh
+++ b/sys/conf/newvers.sh
@@ -36,7 +36,7 @@
 
 TYPE="FreeBSD"
 REVISION="4.3"
-BRANCH="RELEASE-p30"
+BRANCH="RELEASE-p31"
 RELEASE="${REVISION}-${BRANCH}"
 VERSION="${TYPE} ${RELEASE}"