Crash in mail module during SMTP setup

[Maxim Dounin]

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;
 }
 
 
@@ -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/