CVE-2020-7247: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: smtp_mailaddr in smtp_session.c in OpenSMTPD 6.6, as used in OpenBSD 6.6 and other products, allows remote attackers to execute arbitrary commands as root via a crafted SMTP session, as demonstrated by shell metacharacters in a MAIL FROM field. This affects the "uncommented" default configuration. The issue exists because of an incorrect return value upon failure of input validation.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2020-01-29T16:15Z
Repository: https://github.com/openbsd/src
Language: C
Files changed: usr.sbin/smtpd/smtp_session.c
Commit date: 2020-01-28 21:35:00 +0000
Commit message: Fix a security vulnerability discovered by Qualys which can lead to a
privileges escalation on mbox deliveries and unprivileged code execution
on lmtp deliveries, due to a logic issue causing a sanity check to be
missed.

ok eric@, millert@

Diff: diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index f9cf444786dd..1af4c6f9776d 100644
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,6 +1,6 @@
-/*	$OpenBSD: smtp_session.c,v 1.421 2020/01/08 00:05:38 gilles Exp $	*/
+/*	$OpenBSD: smtp_session.c,v 1.422 2020/01/28 21:35:00 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
  * Copyright (c) 2008-2009 Jacek Masiulaniec <jacekm@dobremiasto.net>
@@ -2234,29 +2234,27 @@ smtp_mailaddr(struct mailaddr *maddr, char *line, int mailfrom, char **args,
 	if (p != NULL) {
 		p++;
 		memmove(maddr->user, p, strlen(p) + 1);
 	}
 
-	if (!valid_localpart(maddr->user) ||
-	    !valid_domainpart(maddr->domain)) {
-		/* accept empty return-path in MAIL FROM, required for bounces */
-		if (mailfrom && maddr->user[0] == '\0' && maddr->domain[0] == '\0')
-			return (1);
+	/* accept empty return-path in MAIL FROM, required for bounces */
+	if (mailfrom && maddr->user[0] == '\0' && maddr->domain[0] == '\0')
+		return (1);
 
-		/* no user-part, reject */
-		if (maddr->user[0] == '\0')
-			return (0);
-
-		/* no domain, local user */
-		if (maddr->domain[0] == '\0') {
-			(void)strlcpy(maddr->domain, domain,
-			    sizeof(maddr->domain));
-			return (1);
-		}
+	/* no or invalid user-part, reject */
+	if (maddr->user[0] == '\0' || !valid_localpart(maddr->user))
 		return (0);
+
+	/* no domain part, local user */
+	if (maddr->domain[0] == '\0') {
+		(void)strlcpy(maddr->domain, domain,
+			sizeof(maddr->domain));
 	}
 
+	if (!valid_domainpart(maddr->domain))
+		return (0);
+
 	return (1);
 }
 
 static void
 smtp_cert_init(struct smtp_session *s)

Vulnerable code: /*	$OpenBSD: smtp_session.c,v 1.421 2020/01/08 00:05:38 gilles Exp $	*/
	if (!valid_localpart(maddr->user) ||
	    !valid_domainpart(maddr->domain)) {
		/* accept empty return-path in MAIL FROM, required for bounces */
		if (mailfrom && maddr->user[0] == '\0' && maddr->domain[0] == '\0')
			return (1);
		/* no user-part, reject */
		if (maddr->user[0] == '\0')
			return (0);

		/* no domain, local user */
		if (maddr->domain[0] == '\0') {
			(void)strlcpy(maddr->domain, domain,
			    sizeof(maddr->domain));
			return (1);
		}
Fixed code: /*	$OpenBSD: smtp_session.c,v 1.422 2020/01/28 21:35:00 gilles Exp $	*/
	/* accept empty return-path in MAIL FROM, required for bounces */
	if (mailfrom && maddr->user[0] == '\0' && maddr->domain[0] == '\0')
		return (1);
	/* no or invalid user-part, reject */
	if (maddr->user[0] == '\0' || !valid_localpart(maddr->user))

	/* no domain part, local user */
	if (maddr->domain[0] == '\0') {
		(void)strlcpy(maddr->domain, domain,
			sizeof(maddr->domain));
	if (!valid_domainpart(maddr->domain))
		return (0);