[exim-cvs] Fix crash after TLS channel shutdown

Gitweb: https://git.exim.org/exim.git/commitdiff/bd231acd0f24e4c27c6d6885f48c24360700ec7f

Commit:     bd231acd0f24e4c27c6d6885f48c24360700ec7f
Parent:     1a44d9d799eb1f94d87ae2cef4ca5b31720ccf88
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Sun Jul 28 14:47:29 2019 +0100
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Sun Jul 28 15:55:06 2019 +0100

    Fix crash after TLS channel shutdown
---
 doc/doc-txt/ChangeLog |  4 ++++
 src/src/tls-gnu.c     | 44 +++++++++++++++++++-------------------------
 src/src/tls-openssl.c | 39 ++++++++++++++-------------------------
 src/src/verify.c      |  4 ++--
 4 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 2bfa776..9af9a97 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -157,6 +157,10 @@ JH/33 Bug 2413: Fix dkim_strict option.  Previously the expansion result
       was unused and the unexpanded text used for the test.  Found and
       fixed by Ruben Jenster.

+JH/34 Fix crash after TLS shutdown.  When the TCP/SMTP channel was left open,
+      an attempt to use a TLS library read routine dereffed a nul pointer,
+      causing a segfault.
+

Exim version 4.92
-----------------
diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c
index ca60ddb..de44313 100644
--- a/src/src/tls-gnu.c
+++ b/src/src/tls-gnu.c
@@ -2836,8 +2836,9 @@ void
tls_close(void * ct_ctx, int shutdown)
{
exim_gnutls_state_st * state = ct_ctx ? ct_ctx : &state_server;
+tls_support * tlsp = state->tlsp;

-if (!state->tlsp || state->tlsp->active.sock < 0) return; /* TLS was not active */
+if (!tlsp || tlsp->active.sock < 0) return; /* TLS was not active */

if (shutdown)
{
@@ -2849,12 +2850,26 @@ if (shutdown)
ALARM_CLR(0);
}

+if (!ct_ctx)    /* server */
+  {
+  receive_getc =    smtp_getc;
+  receive_getbuf =    smtp_getbuf;
+  receive_get_cache =    smtp_get_cache;
+  receive_ungetc =    smtp_ungetc;
+  receive_feof =    smtp_feof;
+  receive_ferror =    smtp_ferror;
+  receive_smtp_buffered = smtp_buffered;
+  }
+
 gnutls_deinit(state->session);
 gnutls_certificate_free_credentials(state->x509_cred);

+tlsp->active.sock = -1;
+tlsp->active.tls_ctx = NULL;
+/* Leave bits, peercert, cipher, peerdn, certificate_verified set, for logging */
+tls_channelbinding_b64 = NULL;
+

-state->tlsp->active.sock = -1;
-state->tlsp->active.tls_ctx = NULL;
if (state->xfer_buffer) store_free(state->xfer_buffer);
memcpy(state, &exim_gnutls_state_init, sizeof(exim_gnutls_state_init));
}
@@ -2904,28 +2919,7 @@ if (sigalrm_seen)
else if (inbytes == 0)
{
DEBUG(D_tls) debug_printf("Got TLS_EOF\n");
-
- receive_getc = smtp_getc;
- receive_getbuf = smtp_getbuf;
- receive_get_cache = smtp_get_cache;
- receive_ungetc = smtp_ungetc;
- receive_feof = smtp_feof;
- receive_ferror = smtp_ferror;
- receive_smtp_buffered = smtp_buffered;
-
- gnutls_deinit(state->session);
- gnutls_certificate_free_credentials(state->x509_cred);
-
- state->session = NULL;
- state->tlsp->active.sock = -1;
- state->tlsp->active.tls_ctx = NULL;
- state->tlsp->bits = 0;
- state->tlsp->certificate_verified = FALSE;
- tls_channelbinding_b64 = NULL;
- state->tlsp->cipher = NULL;
- state->tlsp->peercert = NULL;
- state->tlsp->peerdn = NULL;
-
+ tls_close(NULL, TLS_NO_SHUTDOWN);
return FALSE;
}

diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index 9542a1e..e219f5c 100644
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -3189,32 +3189,10 @@ switch(error)
   case SSL_ERROR_ZERO_RETURN:
     DEBUG(D_tls) debug_printf("Got SSL_ERROR_ZERO_RETURN\n");

-    receive_getc = smtp_getc;
-    receive_getbuf = smtp_getbuf;
-    receive_get_cache = smtp_get_cache;
-    receive_ungetc = smtp_ungetc;
-    receive_feof = smtp_feof;
-    receive_ferror = smtp_ferror;
-    receive_smtp_buffered = smtp_buffered;
-
     if (SSL_get_shutdown(server_ssl) == SSL_RECEIVED_SHUTDOWN)
       SSL_shutdown(server_ssl);

-#ifndef DISABLE_OCSP
-    sk_X509_pop_free(server_static_cbinfo->verify_stack, X509_free);
-    server_static_cbinfo->verify_stack = NULL;
-#endif
-    SSL_free(server_ssl);
-    SSL_CTX_free(server_ctx);
-    server_ctx = NULL;
-    server_ssl = NULL;
-    tls_in.active.sock = -1;
-    tls_in.active.tls_ctx = NULL;
-    tls_in.bits = 0;
-    tls_in.cipher = NULL;
-    tls_in.peerdn = NULL;
-    tls_in.sni = NULL;
-
+    tls_close(NULL, TLS_NO_SHUTDOWN);
     return FALSE;

   /* Handle genuine errors */
@@ -3503,14 +3481,25 @@ if (shutdown)
     }
   }

-#ifndef DISABLE_OCSP
 if (!o_ctx)        /* server side */
   {
+#ifndef DISABLE_OCSP
   sk_X509_pop_free(server_static_cbinfo->verify_stack, X509_free);
   server_static_cbinfo->verify_stack = NULL;
-  }
 #endif

+  receive_getc =    smtp_getc;
+  receive_getbuf =    smtp_getbuf;
+  receive_get_cache =    smtp_get_cache;
+  receive_ungetc =    smtp_ungetc;
+  receive_feof =    smtp_feof;
+  receive_ferror =    smtp_ferror;
+  receive_smtp_buffered = smtp_buffered;
+  tls_in.active.tls_ctx = NULL;
+  tls_in.sni = NULL;
+  /* Leave bits, peercert, cipher, peerdn, certificate_verified set, for logging */
+  }
+
 SSL_CTX_free(*ctxp);
 SSL_free(*sslp);
 *ctxp = NULL;
diff --git a/src/src/verify.c b/src/src/verify.c
index e98dee6..a127606 100644
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -1172,7 +1172,7 @@ if (!done)
 /* Come here from within the cache-reading code on fast-track exit. */

 END_CALLOUT:
-tls_modify_variables(&tls_in);
+tls_modify_variables(&tls_in);    /* return variables to inbound values */
 return yield;
 }

@@ -2193,7 +2193,7 @@ the -bv or -bt case). */

 out:
 verify_mode = NULL;
-tls_modify_variables(&tls_in);
+tls_modify_variables(&tls_in);    /* return variables to inbound values */

return yield;
}