From: op <op@openbsd.org>
Date: Wed, 3 Jan 2024 08:11:15 +0000 (+0000)
Subject: relax ORCPT syntax validation
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=cd8603db2179ddaf0e3435f73d0712e5493a5bce;p=openbsd

relax ORCPT syntax validation

We expected the ORCPT parameter to be a valid rfc822 address.  This is
wrong on multiple levels:

 - any other IANA-registered "addr-type" can be used
 - the parameter may be encoded and we didn't decode it prior validation
 - RFC3461 explicitly states that "[..] the address associated with the
   ORCPT keyword is NOT constrained to conform to the syntax rules for
   that 'addr-type'".

Instead, just validate the xtext and preserve the ORCPT value as-is.

Issue originally reported by Tim Kuijsten, Tassilo Philipp and others.

ok millert@
---

diff --git a/usr.sbin/smtpd/envelope.c b/usr.sbin/smtpd/envelope.c
index c9611beb48f..b433096b3a0 100644
--- a/usr.sbin/smtpd/envelope.c
+++ b/usr.sbin/smtpd/envelope.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: envelope.c,v 1.51 2023/02/06 18:35:52 semarie Exp $	*/
+/*	$OpenBSD: envelope.c,v 1.52 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2013 Eric Faurot <eric@openbsd.org>
@@ -443,7 +443,8 @@ ascii_load_field(const char *field, struct envelope *ep, char *buf)
 		return ascii_load_uint8(&ep->dsn_notify, buf);
 
 	if (strcasecmp("dsn-orcpt", field) == 0)
-		return ascii_load_mailaddr(&ep->dsn_orcpt, buf);
+		return ascii_load_string(ep->dsn_orcpt, buf,
+		    sizeof(ep->dsn_orcpt));
 
 	if (strcasecmp("dsn-ret", field) == 0)
 		return ascii_load_dsn_ret(&ep->dsn_ret, buf);
@@ -703,11 +704,8 @@ ascii_dump_field(const char *field, const struct envelope *ep,
 	if (strcasecmp(field, "dsn-ret") == 0)
 		return ascii_dump_dsn_ret(ep->dsn_ret, buf, len);
 
-	if (strcasecmp(field, "dsn-orcpt") == 0) {
-		if (ep->dsn_orcpt.user[0] && ep->dsn_orcpt.domain[0])
-			return ascii_dump_mailaddr(&ep->dsn_orcpt, buf, len);
-		return 1;
-	}
+	if (strcasecmp(field, "dsn-orcpt") == 0)
+		return ascii_dump_string(ep->dsn_orcpt, buf, len);
 
 	if (strcasecmp(field, "dsn-envid") == 0)
 		return ascii_dump_string(ep->dsn_envid, buf, len);
diff --git a/usr.sbin/smtpd/mta.c b/usr.sbin/smtpd/mta.c
index f0bb42c53ff..fe433220ac4 100644
--- a/usr.sbin/smtpd/mta.c
+++ b/usr.sbin/smtpd/mta.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mta.c,v 1.246 2023/11/08 08:46:35 op Exp $	*/
+/*	$OpenBSD: mta.c,v 1.247 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -809,11 +809,8 @@ mta_handle_envelope(struct envelope *evp, const char *smarthost)
 	if (strcmp(buf, e->dest))
 		e->rcpt = xstrdup(buf);
 	e->task = task;
-	if (evp->dsn_orcpt.user[0] && evp->dsn_orcpt.domain[0]) {
-		(void)snprintf(buf, sizeof buf, "%s@%s",
-	    	    evp->dsn_orcpt.user, evp->dsn_orcpt.domain);
-		e->dsn_orcpt = xstrdup(buf);
-	}
+	if (evp->dsn_orcpt[0] != '\0')
+		e->dsn_orcpt = xstrdup(evp->dsn_orcpt);
 	(void)strlcpy(e->dsn_envid, evp->dsn_envid,
 	    sizeof e->dsn_envid);
 	e->dsn_notify = evp->dsn_notify;
diff --git a/usr.sbin/smtpd/mta_session.c b/usr.sbin/smtpd/mta_session.c
index 160b41d30b8..5bdb56c127f 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.149 2023/05/31 16:51:46 op Exp $	*/
+/*	$OpenBSD: mta_session.c,v 1.150 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
@@ -809,7 +809,7 @@ again:
 			    e->dest,
 			    e->dsn_notify ? " NOTIFY=" : "",
 			    e->dsn_notify ? dsn_strnotify(e->dsn_notify) : "",
-			    e->dsn_orcpt ? " ORCPT=rfc822;" : "",
+			    e->dsn_orcpt ? " ORCPT=" : "",
 			    e->dsn_orcpt ? e->dsn_orcpt : "");
 		} else
 			mta_send(s, "RCPT TO:<%s>", e->dest);
diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index f0ede548968..bc048cfe373 100644
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtp_session.c,v 1.438 2023/12/23 10:29:05 op Exp $	*/
+/*	$OpenBSD: smtp_session.c,v 1.439 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -2471,16 +2471,15 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, const char *line)
 				    " combined with other options");
 				return;
 			}
-		} else if (ADVERTISE_EXT_DSN(tx->session) && strncasecmp(opt, "ORCPT=", 6) == 0) {
-			opt += 6;
+		} else if (ADVERTISE_EXT_DSN(tx->session) &&
+		    strncasecmp(opt, "ORCPT=", 6) == 0) {
+			size_t len = sizeof(tx->evp.dsn_orcpt);
 
-			if (strncasecmp(opt, "rfc822;", 7) == 0)
-				opt += 7;
+			opt += 6;
 
-			if (!text_to_mailaddr(&tx->evp.dsn_orcpt, opt) ||
-			    !valid_localpart(tx->evp.dsn_orcpt.user) ||
-			    (strlen(tx->evp.dsn_orcpt.domain) != 0 &&
-			     !valid_domainpart(tx->evp.dsn_orcpt.domain))) {
+			if ((p = strchr(opt, ';')) == NULL ||
+			    !valid_xtext(p + 1) ||
+			    strlcpy(opt, tx->evp.dsn_orcpt, len) >= len) {
 				smtp_reply(tx->session,
 				    "553 ORCPT address syntax error");
 				return;
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 25090c3acf9..5e1b16b6a78 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.679 2023/11/08 08:46:35 op Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.680 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -467,6 +467,7 @@ struct maddrmap {
 #define DSN_NEVER   0x08
 
 #define	DSN_ENVID_LEN	100
+#define	DSN_ORCPT_LEN	500
 
 #define	SMTPD_ENVELOPE_VERSION		3
 struct envelope {
@@ -507,7 +508,7 @@ struct envelope {
 	time_t				nexttry;
 	time_t				lastbounce;
 
-	struct mailaddr			dsn_orcpt;
+	char				dsn_orcpt[DSN_ORCPT_LEN+1];
 	char				dsn_envid[DSN_ENVID_LEN+1];
 	uint8_t				dsn_notify;
 	enum dsn_ret			dsn_ret;
@@ -1703,6 +1704,7 @@ int valid_localpart(const char *);
 int valid_domainpart(const char *);
 int valid_domainname(const char *);
 int valid_smtp_response(const char *);
+int valid_xtext(const char *);
 int secure_file(int, char *, char *, uid_t, int);
 int  lowercase(char *, const char *, size_t);
 void xlowercase(char *, const char *, size_t);
diff --git a/usr.sbin/smtpd/util.c b/usr.sbin/smtpd/util.c
index feb663cc61e..ba9029ee17e 100644
--- a/usr.sbin/smtpd/util.c
+++ b/usr.sbin/smtpd/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.154 2021/06/14 17:58:16 eric Exp $	*/
+/*	$OpenBSD: util.c,v 1.155 2024/01/03 08:11:15 op Exp $	*/
 
 /*
  * Copyright (c) 2000,2001 Markus Friedl.  All rights reserved.
@@ -530,6 +530,30 @@ valid_smtp_response(const char *s)
 	return 1;
 }
 
+int
+valid_xtext(const char *s)
+{
+	for (; *s != '\0'; ++s) {
+		if (*s < '!' || *s > '~' || *s == '=')
+			return 0;
+
+		if (*s != '+')
+			continue;
+
+		s++;
+		if (!isdigit((unsigned char)*s) &&
+		    !(*s >= 'A' && *s <= 'F'))
+			return 0;
+
+		s++;
+		if (!isdigit((unsigned char)*s) &&
+		    !(*s >= 'A' && *s <= 'F'))
+			return 0;
+	}
+
+	return 1;
+}
+
 int
 secure_file(int fd, char *path, char *userdir, uid_t uid, int mayread)
 {