1
0
mirror of https://github.com/openbsd/src.git synced 2025-01-10 06:47:55 -08:00

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@
This commit is contained in:
gilles 2024-02-02 22:02:12 +00:00
parent 35b7f40312
commit 03e780bb85
3 changed files with 20 additions and 13 deletions

View File

@ -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> * 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; break;
} }
xn->realuser = 1; xn->realuser = 1;
xn->realuser_uid = lk.userinfo.uid;
if (xn->sameuser && xn->parent->forwarded) { if (xn->sameuser && xn->parent->forwarded) {
log_trace(TRACE_EXPAND, "expand: lka_expand: same " 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; break;
case EXPAND_FILENAME: 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); dsp = dict_xget(env->sc_dispatchers, rule->dispatcher);
if (dsp->u.local.forward_only) { if (dsp->u.local.forward_only) {
log_trace(TRACE_EXPAND, "expand: filename matched on forward-only rule"); 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; break;
case EXPAND_FILTER: 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); dsp = dict_xget(env->sc_dispatchers, rule->dispatcher);
if (dsp->u.local.forward_only) { if (dsp->u.local.forward_only) {
log_trace(TRACE_EXPAND, "expand: filter matched on forward-only rule"); log_trace(TRACE_EXPAND, "expand: filter matched on forward-only rule");

View File

@ -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> * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@ -1425,15 +1425,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver *deliver)
pw_dir = deliver->userinfo.directory; pw_dir = deliver->userinfo.directory;
} }
if (pw_uid == 0 && deliver->mda_exec[0]) { if (pw_uid == 0 && (!dsp->u.local.is_mbox || deliver->mda_exec[0])) {
pw_name = deliver->userinfo.username; (void)snprintf(ebuf, sizeof ebuf, "MDA not allowed to deliver to: %s",
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); deliver->userinfo.username);
m_create(p_dispatcher, IMSG_MDA_DONE, 0, 0, -1); m_create(p_dispatcher, IMSG_MDA_DONE, 0, 0, -1);
m_add_id(p_dispatcher, id); m_add_id(p_dispatcher, id);

View File

@ -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> * Copyright (c) 2008 Gilles Chehade <gilles@poolp.org>
@ -428,6 +428,7 @@ struct expandnode {
enum expand_type type; enum expand_type type;
int sameuser; int sameuser;
int realuser; int realuser;
uid_t realuser_uid;
int forwarded; int forwarded;
struct rule *rule; struct rule *rule;
struct expandnode *parent; struct expandnode *parent;