Fix two security vulnerabilities discovered by Qualys.

An out of bounds read in smtpd allows an attacker to inject arbitrary
commands into the envelope file which are then executed as root.
Separately, missing privilege revocation in smtpctl allows arbitrary
commands to be run with the _smtpq group.

6 files changed, 47 insertions, 23 deletions

diff --git a/usr.sbin/smtpd/makemap.c b/usr.sbin/smtpd/makemap.c
index dd8bc8d300e..1352c628b3e 100644
--- a/usr.sbin/smtpd/makemap.c
+++ b/usr.sbin/smtpd/makemap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: makemap.c,v 1.72 2018/12/28 11:40:29 eric Exp $	*/
+/*	$OpenBSD: makemap.c,v 1.73 2020/02/24 16:16:07 millert Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -83,8 +83,13 @@ makemap(int prog_mode, int argc, char *argv[])
 	int		 ch, dbputs = 0, Uflag = 0;
 	DBTYPE		 dbtype = DB_HASH;
 	char		*p;
+	gid_t		 gid;
 	int		 fd = -1;
 
+	gid = getgid();
+	if (setresgid(gid, gid, gid) == -1)
+		err(1, "setresgid");
+
 	if ((env = config_default()) == NULL)
 		err(1, NULL);
 
@@ -161,9 +166,9 @@ makemap(int prog_mode, int argc, char *argv[])
 				errx(1, "database name too long");
 		}
 
-		execlp("makemap", "makemap", "-d", argv[0], "-o", dbname, "-",
-		    (char *)NULL);
-		err(1, "execlp");
+		execl(PATH_MAKEMAP, "makemap", "-d", argv[0], "-o", dbname,
+		    "-", (char *)NULL);
+		err(1, "execl");
 	}
 
 	if (mode == P_NEWALIASES) {
diff --git a/usr.sbin/smtpd/mta_session.c b/usr.sbin/smtpd/mta_session.c
index b57a93d62a2..3f85566402b 100644
--- a/usr.sbin/smtpd/mta_session.c
+++ b/usr.sbin/smtpd/mta_session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mta_session.c,v 1.131 2020/02/03 15:53:52 gilles Exp $	*/
+/*	$OpenBSD: mta_session.c,v 1.132 2020/02/24 16:16:07 millert Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -1294,7 +1294,7 @@ mta_io(struct io *io, int evt, void *arg)
 		if (cont) {
 			if (s->replybuf[0] == '\0')
 				(void)strlcat(s->replybuf, line, sizeof s->replybuf);
-			else {
+			else if (len > 4) {
 				line = line + 4;
 				if (isdigit((int)*line) && *(line + 1) == '.' &&
 				    isdigit((int)*line+2) && *(line + 3) == '.' &&
@@ -1309,7 +1309,9 @@ mta_io(struct io *io, int evt, void *arg)
 		/* last line of a reply, check if we're on a continuation to parse out status and ESC.
 		 * if we overflow reply buffer or are not on continuation, log entire last line.
 		 */
-		if (s->replybuf[0] != '\0') {
+		if (s->replybuf[0] == '\0')
+			(void)strlcat(s->replybuf, line, sizeof s->replybuf);
+		else if (len > 4) {
 			p = line + 4;
 			if (isdigit((int)*p) && *(p + 1) == '.' &&
 			    isdigit((int)*p+2) && *(p + 3) == '.' &&
@@ -1318,8 +1320,6 @@ mta_io(struct io *io, int evt, void *arg)
 			if (strlcat(s->replybuf, p, sizeof s->replybuf) >= sizeof s->replybuf)
 				(void)strlcpy(s->replybuf, line, sizeof s->replybuf);
 		}
-		else
-			(void)strlcpy(s->replybuf, line, sizeof s->replybuf);
 
 		if (s->state == MTA_QUIT) {
 			log_info("%016"PRIx64" mta disconnected reason=quit messages=%zu",
diff --git a/usr.sbin/smtpd/smtpctl.c b/usr.sbin/smtpd/smtpctl.c
index 129abb51835..bd54cc3a914 100644
--- a/usr.sbin/smtpd/smtpctl.c
+++ b/usr.sbin/smtpd/smtpctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpctl.c,v 1.166 2020/01/06 11:03:06 gilles Exp $	*/
+/*	$OpenBSD: smtpctl.c,v 1.167 2020/02/24 16:16:07 millert Exp $	*/
 
 /*
  * Copyright (c) 2013 Eric Faurot <eric@openbsd.org>
@@ -1125,7 +1125,7 @@ sendmail_compat(int argc, char **argv)
 		 */
 		for (i = 1; i < argc; i++)
 			if (strncmp(argv[i], "-bi", 3) == 0)
-				exit(makemap(P_NEWALIASES, argc, argv));
+				exit(makemap(P_SENDMAIL, argc, argv));
 
 		if (!srv_connect())
 			offlinefp = offline_file();
diff --git a/usr.sbin/smtpd/smtpd-defines.h b/usr.sbin/smtpd/smtpd-defines.h
index abdb208e7c7..279d9e27b1e 100644
--- a/usr.sbin/smtpd/smtpd-defines.h
+++ b/usr.sbin/smtpd/smtpd-defines.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd-defines.h,v 1.11 2020/01/31 22:01:20 gilles Exp $	*/
+/*	$OpenBSD: smtpd-defines.h,v 1.12 2020/02/24 16:16:08 millert Exp $	*/
 
 /*
  * Copyright (c) 2013 Gilles Chehade <gilles@poolp.org>
@@ -38,6 +38,7 @@
 #define SMTPD_QUEUE_GROUP	"_smtpq"
 #define PATH_SPOOL		"/var/spool/smtpd"
 #define	PATH_MAILLOCAL		"/usr/libexec/mail.local"
+#define PATH_MAKEMAP		"/usr/sbin/makemap"
 
 #define SUBADDRESSING_DELIMITER	"+"
 
diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index ada126cbe02..ce1262fa4fc 100644
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.c,v 1.331 2020/02/03 15:41:22 gilles Exp $	*/
+/*	$OpenBSD: smtpd.c,v 1.332 2020/02/24 16:16:08 millert Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -81,9 +81,9 @@ static struct mproc *setup_peer(enum smtp_proc_type, pid_t, int);
 static int imsg_wait(struct imsgbuf *, struct imsg *, int);
 
 static void	offline_scan(int, short, void *);
-static int	offline_add(char *);
+static int	offline_add(char *, uid_t, gid_t);
 static void	offline_done(void);
-static int	offline_enqueue(char *);
+static int	offline_enqueue(char *, uid_t, gid_t);
 
 static void	purge_task(void);
 static int	parent_auth_user(const char *, const char *);
@@ -112,6 +112,8 @@ struct child {
 
 struct offline {
 	TAILQ_ENTRY(offline)	 entry;
+	uid_t			 uid;
+	gid_t			 gid;
 	char			*path;
 };
 
@@ -1585,7 +1587,8 @@ offline_scan(int fd, short ev, void *arg)
 			continue;
 		}
 
-		if (offline_add(e->fts_name)) {
+		if (offline_add(e->fts_name, e->fts_statp->st_uid,
+		    e->fts_statp->st_gid)) {
 			log_warnx("warn: smtpd: "
 			    "could not add offline message %s", e->fts_name);
 			continue;
@@ -1605,7 +1608,7 @@ offline_scan(int fd, short ev, void *arg)
 }
 
 static int
-offline_enqueue(char *name)
+offline_enqueue(char *name, uid_t uid, gid_t gid)
 {
 	char		*path;
 	struct stat	 sb;
@@ -1668,6 +1671,18 @@ offline_enqueue(char *name)
 			_exit(1);
 		}
 
+		if (sb.st_uid != uid) {
+			log_warnx("warn: smtpd: file %s has bad uid %d",
+			    path, sb.st_uid);
+			_exit(1);
+		}
+
+		if (sb.st_gid != gid) {
+			log_warnx("warn: smtpd: file %s has bad gid %d",
+			    path, sb.st_gid);
+			_exit(1);
+		}
+
 		pw = getpwuid(sb.st_uid);
 		if (pw == NULL) {
 			log_warnx("warn: smtpd: getpwuid for uid %d failed",
@@ -1724,17 +1739,19 @@ offline_enqueue(char *name)
 }
 
 static int
-offline_add(char *path)
+offline_add(char *path, uid_t uid, gid_t gid)
 {
 	struct offline	*q;
 
 	if (offline_running < OFFLINE_QUEUEMAX)
 		/* skip queue */
-		return offline_enqueue(path);
+		return offline_enqueue(path, uid, gid);
 
 	q = malloc(sizeof(*q) + strlen(path) + 1);
 	if (q == NULL)
 		return (-1);
+	q->uid = uid;
+	q->gid = gid;
 	q->path = (char *)q + sizeof(*q);
 	memmove(q->path, path, strlen(path) + 1);
 	TAILQ_INSERT_TAIL(&offline_q, q, entry);
@@ -1753,7 +1770,7 @@ offline_done(void)
 		if ((q = TAILQ_FIRST(&offline_q)) == NULL)
 			break; /* all done */
 		TAILQ_REMOVE(&offline_q, q, entry);
-		offline_enqueue(q->path);
+		offline_enqueue(q->path, q->uid, q->gid);
 		free(q);
 	}
 }
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index c7952ff3129..8f8fbc8ac95 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.653 2020/02/03 15:41:22 gilles Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.654 2020/02/24 16:16:08 millert Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -98,8 +98,9 @@
 #define MTA_EXT_DSN		0x400
 
 
-#define P_NEWALIASES	0
-#define P_MAKEMAP	1
+#define P_SENDMAIL	0
+#define P_NEWALIASES	1
+#define P_MAKEMAP	2
 
 #define	CERT_ERROR	-1
 #define	CERT_OK		 0