From 03e780bb85dd986ebd9fbd8799d27f3e6534f196 Mon Sep 17 00:00:00 2001 From: gilles Date: Fri, 2 Feb 2024 22:02:12 +0000 Subject: [PATCH] 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@ --- usr.sbin/smtpd/lka_session.c | 15 ++++++++++++++- usr.sbin/smtpd/smtpd.c | 15 ++++----------- usr.sbin/smtpd/smtpd.h | 3 ++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/usr.sbin/smtpd/lka_session.c b/usr.sbin/smtpd/lka_session.c index 60429999a16..a62f6c6bb0e 100644 --- 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 @@ -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 024b9b1ea23..789aa17968f 100644 --- 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 @@ -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 5e1b16b6a78..a0e8add90a8 100644 --- 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 @@ -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; -- 2.20.1