Crash in mail module during SMTP setup

Maxim Dounin mdounin at mdounin.ru
Tue Jul 30 16:11:32 UTC 2019


Hello!

On Tue, Jul 30, 2019 at 06:32:43PM +0300, Maxim Dounin wrote:

> Hello!
> 
> On Tue, Jul 30, 2019 at 10:39:56PM +1000, Rob N ★ wrote:
> 
> > On Tue, 30 Jul 2019, at 4:26 AM, Maxim Dounin wrote:
> > > Looking at "p *c" and "p *s" might be also interesting.
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00000000005562f2 in ngx_mail_smtp_resolve_name_handler (ctx=0x7bcaa40)
> >  at src/mail/ngx_mail_smtp_handler.c:215
> > 215 ngx_log_debug1(NGX_LOG_DEBUG_MAIL, c->log, 0,
> > 
> > (gdb) p *c
> > $14 = {data = 0x30, read = 0x111, write = 0xc2cfff0, fd = 263201712,
> >  recv = 0xfb023c0, send = 0x0, recv_chain = 0xb0, send_chain = 0x350cf90,
> >  listening = 0x0, sent = 55627856, log = 0x0, pool = 0x350cff0,
> >  type = -1242759166, sockaddr = 0x0, socklen = 7, addr_text = {len = 0,
> >  data = 0x2c4e8fc ""}, proxy_protocol_addr = {len = 0,
> >  data = 0x54eb79 <ngx_mail_log_error> "UH\211\345H\203\354 at H\211}\330H\211u\320H\211U\310H\213E\330H\213@@H\205\300tCH\213E\330H\213P at H\213u\310H\213E\320H\211\321\272\234\064z"}, proxy_protocol_port = 53344,
> >  ssl = 0x484cb1 <ngx_syslog_writer>, udp = 0x2018d20,
> >  local_sockaddr = 0x7a414a, local_socklen = 0, buffer = 0x33312e32322e3438,
> >  queue = {prev = 0x3031312e36, next = 0x0}, number = 204275712,
> >  requests = 139872032560632, buffered = 0, log_error = 0, timedout = 0,
> >  error = 0, destroyed = 0, idle = 0, reusable = 0, close = 1, shared = 0,
> >  sendfile = 1, sndlowat = 1, tcp_nodelay = 2, tcp_nopush = 0,
> >  need_last_buf = 0}
> 
> It looks like "c" points to garbage.  
> 
> > 
> > (gdb) p *s
> > $15 = {signature = 155588656, connection = 0x350cf80, out = {len = 35,
> 
> Signature should be 0x4C49414D ("MAIL") == 1279869261, so this 
> looks like garbage too.  And this explains why "c" points to 
> garbage.
> 
> >  data = 0x20ae3e0 "220 smtp.fastmail.com ESMTP ready\r\n250 smtp.fastmail.com\r\n250-smtp.fastmail.com\r\n250-PIPELINING\r\n250-SIZE 71000000\r\n250-ENHANCEDSTATUSCODES\r\n250-8BITMIME\r\n250-AUTH PLAIN LOGIN\r\n250 AUTH=PLAIN LOGIN\r\n2"...}, buffer = 0x0, ctx = 0xfb02470, main_conf = 0x2015218,
> 
> Except there are some seemingly valid fields - it looks like 
> s->out is set to sscf->greeting.  So it looks like this might be 
> an already closed and partially overwritten session.
> 
> Given that "s->out = sscf->greeting;" is expected to happen after 
> client address resolution, likely this is a duplicate handler call 
> from the resolver.
> 
> I think I see the problem - when using SMTP with SSL and resolver, 
> read events might be enabled during address resolving, leading to 
> duplicate ngx_mail_ssl_handshake_handler() calls if something 
> arrives from the client, and duplicate session initialization - 
> including starting another resolving.
> 
> The following patch should resolve this:
> 
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1564500680 -10800
> #      Tue Jul 30 18:31:20 2019 +0300
> # Node ID 63604bfd60a09c7c91ce62c89df468a6e54d2f1c
> # Parent  e7181cfe9212de7f67df805bb746519c059b490b
> Mail: fixed duplicate resolving.
> 
> When using SMTP with SSL and resolver, read events might be enabled
> during address resolving, leading to duplicate ngx_mail_ssl_handshake_handler()
> calls if something arrives from the client, and duplicate session
> initialization - including starting another resolving.  This can lead
> to a segmentation fault if the session is closed after first resolving
> finished.  Fix is to block read events while resolving.
> 
> Reported by Robert Norris,
> http://mailman.nginx.org/pipermail/nginx/2019-July/058204.html.
> 
> diff --git a/src/mail/ngx_mail_smtp_handler.c b/src/mail/ngx_mail_smtp_handler.c
> --- a/src/mail/ngx_mail_smtp_handler.c
> +++ b/src/mail/ngx_mail_smtp_handler.c
> @@ -15,6 +15,7 @@
>  static void ngx_mail_smtp_resolve_addr_handler(ngx_resolver_ctx_t *ctx);
>  static void ngx_mail_smtp_resolve_name(ngx_event_t *rev);
>  static void ngx_mail_smtp_resolve_name_handler(ngx_resolver_ctx_t *ctx);
> +static void ngx_mail_smtp_block_reading(ngx_event_t *rev);
>  static void ngx_mail_smtp_greeting(ngx_mail_session_t *s, ngx_connection_t *c);
>  static void ngx_mail_smtp_invalid_pipelining(ngx_event_t *rev);
>  static ngx_int_t ngx_mail_smtp_create_buffer(ngx_mail_session_t *s,
> @@ -91,6 +92,9 @@ ngx_mail_smtp_init_session(ngx_mail_sess
>      if (ngx_resolve_addr(ctx) != NGX_OK) {
>          ngx_mail_close_connection(c);
>      }
> +
> +    s->resolver_ctx = ctx;
> +    c->read->handler = ngx_mail_smtp_block_reading;
>  }
>  
>  
> @@ -172,6 +176,9 @@ ngx_mail_smtp_resolve_name(ngx_event_t *
>      if (ngx_resolve_name(ctx) != NGX_OK) {
>          ngx_mail_close_connection(c);
>      }
> +
> +    s->resolver_ctx = ctx;
> +    c->read->handler = ngx_mail_smtp_block_reading;
>  }

Err, this should be before ngx_resolve_addr()/ngx_resolve_name().
Updated patch:

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1564502955 -10800
#      Tue Jul 30 19:09:15 2019 +0300
# Node ID 9744505242b6ba59f0a8752e52c6e73050dd1cc6
# Parent  d11673c35dc184a7030c9b678d3ad89376dd3079
Mail: fixed duplicate resolving.

When using SMTP with SSL and resolver, read events might be enabled
during address resolving, leading to duplicate ngx_mail_ssl_handshake_handler()
calls if something arrives from the client, and duplicate session
initialization - including starting another resolving.  This can lead
to a segmentation fault if the session is closed after first resolving
finished.  Fix is to block read events while resolving.

Reported by Robert Norris,
http://mailman.nginx.org/pipermail/nginx/2019-July/058204.html.

diff --git a/src/mail/ngx_mail_smtp_handler.c b/src/mail/ngx_mail_smtp_handler.c
--- a/src/mail/ngx_mail_smtp_handler.c
+++ b/src/mail/ngx_mail_smtp_handler.c
@@ -15,6 +15,7 @@
 static void ngx_mail_smtp_resolve_addr_handler(ngx_resolver_ctx_t *ctx);
 static void ngx_mail_smtp_resolve_name(ngx_event_t *rev);
 static void ngx_mail_smtp_resolve_name_handler(ngx_resolver_ctx_t *ctx);
+static void ngx_mail_smtp_block_reading(ngx_event_t *rev);
 static void ngx_mail_smtp_greeting(ngx_mail_session_t *s, ngx_connection_t *c);
 static void ngx_mail_smtp_invalid_pipelining(ngx_event_t *rev);
 static ngx_int_t ngx_mail_smtp_create_buffer(ngx_mail_session_t *s,
@@ -88,6 +89,9 @@ ngx_mail_smtp_init_session(ngx_mail_sess
     ctx->data = s;
     ctx->timeout = cscf->resolver_timeout;
 
+    s->resolver_ctx = ctx;
+    c->read->handler = ngx_mail_smtp_block_reading;
+
     if (ngx_resolve_addr(ctx) != NGX_OK) {
         ngx_mail_close_connection(c);
     }
@@ -169,6 +173,9 @@ ngx_mail_smtp_resolve_name(ngx_event_t *
     ctx->data = s;
     ctx->timeout = cscf->resolver_timeout;
 
+    s->resolver_ctx = ctx;
+    c->read->handler = ngx_mail_smtp_block_reading;
+
     if (ngx_resolve_name(ctx) != NGX_OK) {
         ngx_mail_close_connection(c);
     }
@@ -239,6 +246,38 @@ found:
 
 
 static void
+ngx_mail_smtp_block_reading(ngx_event_t *rev)
+{
+    ngx_connection_t    *c;
+    ngx_mail_session_t  *s;
+    ngx_resolver_ctx_t  *ctx;
+
+    c = rev->data;
+    s = c->data;
+
+    ngx_log_debug0(NGX_LOG_DEBUG_MAIL, c->log, 0,
+                   "smtp reading blocked");
+
+    if (ngx_handle_read_event(rev, 0) != NGX_OK) {
+        if (s->resolver_ctx) {
+            ctx = s->resolver_ctx;
+
+            if (ctx->handler == ngx_mail_smtp_resolve_addr_handler) {
+                ngx_resolve_addr_done(ctx);
+
+            } else if (ctx->handler == ngx_mail_smtp_resolve_name_handler) {
+                ngx_resolve_name_done(ctx);
+            }
+
+            s->resolver_ctx = NULL;
+        }
+
+        ngx_mail_close_connection(c);
+    }
+}
+
+
+static void
 ngx_mail_smtp_greeting(ngx_mail_session_t *s, ngx_connection_t *c)
 {
     ngx_msec_t                 timeout;
@@ -258,6 +297,10 @@ ngx_mail_smtp_greeting(ngx_mail_session_
         ngx_mail_close_connection(c);
     }
 
+    if (c->read->ready) {
+        ngx_post_event(c->read, &ngx_posted_events);
+    }
+
     if (sscf->greeting_delay) {
          c->read->handler = ngx_mail_smtp_invalid_pipelining;
          return;
-- 
Maxim Dounin
http://mdounin.ru/


More information about the nginx mailing list