diff options
author | Eric Faurot <eric@cvs.openbsd.org> | 2013-02-21 16:25:22 +0000 |
---|---|---|
committer | Eric Faurot <eric@cvs.openbsd.org> | 2013-02-21 16:25:22 +0000 |
commit | 28111057c651d1bc9e403860f892b391da90928c (patch) | |
tree | b6a2bb9586d2e1600091adfbbf03d03f29d31ce5 /usr.sbin | |
parent | 7b7333b7c94760985729e220afa82c0a3eb442cb (diff) |
Fix a potential crash when connecting to a misbehaving smtp server.
If a smtp session got bogus data from a remote server and has just
issued an internal query, then defer the deletion of that session
until it gets the reply.
ok gilles@ deraadt@
Diffstat (limited to 'usr.sbin')
-rw-r--r-- | usr.sbin/smtpd/mta_session.c | 55 |
1 files changed, 47 insertions, 8 deletions
diff --git a/usr.sbin/smtpd/mta_session.c b/usr.sbin/smtpd/mta_session.c index 65665867ebf..49abb2c62e7 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.33 2013/02/15 22:43:21 eric Exp $ */ +/* $OpenBSD: mta_session.c,v 1.34 2013/02/21 16:25:21 eric Exp $ */ /* * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org> @@ -83,8 +83,8 @@ enum mta_state { #define MTA_VERIFIED 0x0200 #define MTA_FREE 0x0400 - #define MTA_LMTP 0x0800 +#define MTA_WAIT 0x1000 #define MTA_EXT_STARTTLS 0x01 #define MTA_EXT_AUTH 0x02 @@ -134,6 +134,7 @@ static const char * mta_strstate(int); static int mta_check_loop(FILE *); static void mta_start_tls(struct mta_session *); static int mta_verify_certificate(struct mta_session *); +static struct mta_session *mta_tree_pop(struct tree *, uint64_t); static struct tree wait_helo; static struct tree wait_ptr; @@ -211,6 +212,7 @@ mta_session(struct mta_relay *relay, struct mta_route *route) } else if (waitq_wait(&route->dst->ptrname, mta_on_ptr, s)) { dns_query_ptr(s->id, s->route->dst->sa); tree_xset(&wait_ptr, s->id, s); + s->flags |= MTA_WAIT; } } @@ -236,7 +238,12 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) if (imsg->fd == -1) fatalx("mta: cannot obtain msgfd"); - s = tree_xpop(&wait_fd, reqid); + s = mta_tree_pop(&wait_fd, reqid); + if (s == NULL) { + close(imsg->fd); + return; + } + s->datafp = fdopen(imsg->fd, "r"); if (s->datafp == NULL) fatal("mta: fdopen"); @@ -263,7 +270,10 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) else m_get_string(&m, &name); m_end(&m); - s = tree_xpop(&wait_ptr, reqid); + s = mta_tree_pop(&wait_ptr, reqid); + if (s == NULL) + return; + h = s->route->dst; h->lastptrquery = time(NULL); if (name) @@ -273,7 +283,9 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) case IMSG_LKA_SSL_INIT: resp_ca_cert = imsg->data; - s = tree_xpop(&wait_ssl_init, resp_ca_cert->reqid); + s = mta_tree_pop(&wait_ssl_init, resp_ca_cert->reqid); + if (s == NULL) + return; if (resp_ca_cert->status == CA_FAIL) { log_info("smtp-out: Disconnecting session %016" PRIx64 @@ -306,7 +318,9 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) case IMSG_LKA_SSL_VERIFY: resp_ca_vrfy = imsg->data; - s = tree_xpop(&wait_ssl_verify, resp_ca_vrfy->reqid); + s = mta_tree_pop(&wait_ssl_verify, resp_ca_vrfy->reqid); + if (s == NULL) + return; if (resp_ca_vrfy->status == CA_OK) s->flags |= MTA_VERIFIED; @@ -324,7 +338,9 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) m_get_string(&m, &name); m_end(&m); - s = tree_xpop(&wait_helo, reqid); + s = mta_tree_pop(&wait_helo, reqid); + if (s == NULL) + return; if (status == LKA_OK) { s->helo = xstrdup(name, "mta_session_imsg"); @@ -342,6 +358,22 @@ mta_session_imsg(struct mproc *p, struct imsg *imsg) } } +static struct mta_session * +mta_tree_pop(struct tree *wait, uint64_t reqid) +{ + struct mta_session *s; + + s = tree_xpop(wait, reqid); + if (s->flags & MTA_FREE) { + log_debug("debug: mta: %p: zombie session", s); + mta_free(s); + return (NULL); + } + s->flags &= ~MTA_WAIT; + + return (s); +} + static void mta_free(struct mta_session *s) { @@ -399,6 +431,7 @@ mta_connect(struct mta_session *s) m_add_sockaddr(p_lka, s->route->src->sa); m_close(p_lka); tree_xset(&wait_helo, s->id, s); + s->flags |= MTA_WAIT; return; } if (s->relay->heloname) @@ -581,6 +614,7 @@ mta_enter_state(struct mta_session *s, int newstate) m_close(p_queue); tree_xset(&wait_fd, s->id, s); + s->flags |= MTA_WAIT; break; case MTA_MAIL: @@ -921,7 +955,10 @@ mta_io(struct io *io, int evt) if (iobuf_len(&s->iobuf)) { log_debug("debug: mta: remaining data in input buffer"); mta_error(s, "Remote host sent too much data"); - mta_free(s); + if (s->flags & MTA_WAIT) + s->flags |= MTA_FREE; + else + mta_free(s); } break; @@ -1156,6 +1193,7 @@ mta_start_tls(struct mta_session *s) m_compose(p_lka, IMSG_LKA_SSL_INIT, 0, 0, -1, &req_ca_cert, sizeof(req_ca_cert)); tree_xset(&wait_ssl_init, s->id, s); + s->flags |= MTA_WAIT; return; } ssl = ssl_mta_init(NULL, 0, NULL, 0); @@ -1187,6 +1225,7 @@ mta_verify_certificate(struct mta_session *s) */ tree_xset(&wait_ssl_verify, s->id, s); + s->flags |= MTA_WAIT; /* Send the client certificate */ bzero(&req_ca_vrfy, sizeof req_ca_vrfy); |