there's no good reason to allow smtpd to execute custom command set by root

in a .forward file so disallow custom commands and file reading, only allow
setting forward addresses and users.

as root is no longer allowed to run any MDA but mbox, we can be stricter on
the setup of the MDA process and refuse to exec anything that's not an mbox
dispatcher.

tested by op@ who edited a root envelope to simulate an exploit injecting a
custom command in a root envelope, smtpd refused to exec.

ok millert@ and op@

diff --git a/usr.sbin/smtpd/lka_session.c b/usr.sbin/smtpd/lka_session.c
index 6042999..a62f6c6 100644 (file)
--- a/usr.sbin/smtpd/lka_session.c
+++ b/usr.sbin/smtpd/lka_session.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: lka_session.c,v 1.98 2023/11/03 13:40:07 op Exp $     */
+/*     $OpenBSD: lka_session.c,v 1.99 2024/02/02 22:02:12 gilles Exp $ */
 
 /*
  * Copyright (c) 2011 Gilles Chehade <gilles@poolp.org>
@@ -397,6 +397,7 @@ lka_expand(struct lka_session *lks, struct rule *rule, struct expandnode *xn)
                        break;
                }
                xn->realuser = 1;
+               xn->realuser_uid = lk.userinfo.uid;
 
                if (xn->sameuser && xn->parent->forwarded) {
                        log_trace(TRACE_EXPAND, "expand: lka_expand: same "
@@ -423,6 +424,12 @@ lka_expand(struct lka_session *lks, struct rule *rule, struct expandnode *xn)
                break;
 
        case EXPAND_FILENAME:
+               if (xn->parent->realuser && xn->parent->realuser_uid == 0) {
+                       log_trace(TRACE_EXPAND, "expand: filename not allowed in root's forward");
+                       lks->error = LKA_TEMPFAIL;
+                       break;
+               }
+
                dsp = dict_xget(env->sc_dispatchers, rule->dispatcher);
                if (dsp->u.local.forward_only) {
                        log_trace(TRACE_EXPAND, "expand: filename matched on forward-only rule");
@@ -451,6 +458,12 @@ lka_expand(struct lka_session *lks, struct rule *rule, struct expandnode *xn)
                break;
 
        case EXPAND_FILTER:
+               if (xn->parent->realuser && xn->parent->realuser_uid == 0) {
+                       log_trace(TRACE_EXPAND, "expand: filter not allowed in root's forward");
+                       lks->error = LKA_TEMPFAIL;
+                       break;
+               }
+
                dsp = dict_xget(env->sc_dispatchers, rule->dispatcher);
                if (dsp->u.local.forward_only) {
                        log_trace(TRACE_EXPAND, "expand: filter matched on forward-only rule");

diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index 024b9b1..789aa17 100644 (file)
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtpd.c,v 1.347 2024/01/20 09:01:03 claudio Exp $     */
+/*     $OpenBSD: smtpd.c,v 1.348 2024/02/02 22:02:12 gilles Exp $      */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -1425,16 +1425,9 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver)
                pw_dir = deliver->userinfo.directory;
        }
 
-       if (pw_uid == 0 && deliver->mda_exec[0]) {
-               pw_name = deliver->userinfo.username;
-               pw_uid = deliver->userinfo.uid;
-               pw_gid = deliver->userinfo.gid;
-               pw_dir = deliver->userinfo.directory;
-       }
-
-       if (pw_uid == 0 && !dsp->u.local.is_mbox) {
-               (void)snprintf(ebuf, sizeof ebuf, "not allowed to deliver to: %s",
-                   deliver->userinfo.username);
+       if (pw_uid == 0 && (!dsp->u.local.is_mbox || deliver->mda_exec[0])) {
+               (void)snprintf(ebuf, sizeof ebuf, "MDA not allowed to deliver to: %s",
+                              deliver->userinfo.username);
                m_create(p_dispatcher, IMSG_MDA_DONE, 0, 0, -1);
                m_add_id(p_dispatcher, id);
                m_add_int(p_dispatcher, MDA_PERMFAIL);

diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 5e1b16b..a0e8add 100644 (file)
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: smtpd.h,v 1.680 2024/01/03 08:11:15 op Exp $  */
+/*     $OpenBSD: smtpd.h,v 1.681 2024/02/02 22:02:12 gilles Exp $      */
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@@ -428,6 +428,7 @@ struct expandnode {
        enum expand_type        type;
        int                     sameuser;
        int                     realuser;
+       uid_t                   realuser_uid;
        int                     forwarded;
        struct rule            *rule;
        struct expandnode      *parent;