commit 1b7463478e30c6855ea62db1f5647630f0acd24b
author Pierre Ossman <ossman@cendio.se> Fri Sep 21 15:33:30 2018 +0200
committer Pierre Ossman <ossman@cendio.se> Fri Sep 21 15:59:46 2018 +0200
tree b0d2331f8b4204a830ba0fa8c7072033f04e4f0d
parent ad2b3c4aa3db056b83f5d91564400e03f3239835

Delete TLS streams before deleting the session

The streams depend on the session and can crash the program if they
are removed in the wrong order. Do a general cleanup of the life time
management of the streams.

common/rfb/SSecurityTLS.cxx

diff --git a/common/rfb/SSecurityTLS.cxx b/common/rfb/SSecurityTLS.cxx
index 72b83db..bf77b9b 100644
--- a/common/rfb/SSecurityTLS.cxx
+++ b/common/rfb/SSecurityTLS.cxx
@@ -51,7 +51,7 @@
 
 SSecurityTLS::SSecurityTLS(SConnection* sc, bool _anon)
   : SSecurity(sc), session(NULL), dh_params(NULL), anon_cred(NULL),
-    cert_cred(NULL), anon(_anon), fis(NULL), fos(NULL)
+    cert_cred(NULL), anon(_anon), tlsis(NULL), tlsos(NULL)
 {
   certfile = X509_CertFile.getData();
   keyfile = X509_KeyFile.getData();
@@ -84,6 +84,15 @@
     cert_cred = 0;
   }
 
+  if (tlsis) {
+    delete tlsis;
+    tlsis = NULL;
+  }
+  if (tlsos) {
+    delete tlsos;
+    tlsos = NULL;
+  }
+
   if (session) {
     gnutls_deinit(session);
     session = 0;
@@ -95,11 +104,6 @@
 {
   shutdown();
 
-  if (fis)
-    delete fis;
-  if (fos)
-    delete fos;
-
   delete[] keyfile;
   delete[] certfile;
 
@@ -108,12 +112,12 @@
 
 bool SSecurityTLS::processMsg()
 {
-  rdr::InStream* is = sc->getInStream();
-  rdr::OutStream* os = sc->getOutStream();
-
   vlog.debug("Process security message (session %p)", session);
 
   if (!session) {
+    rdr::InStream* is = sc->getInStream();
+    rdr::OutStream* os = sc->getOutStream();
+
     if (gnutls_init(&session, GNUTLS_SERVER) != GNUTLS_E_SUCCESS)
       throw AuthFailureException("gnutls_init failed");
 
@@ -130,17 +134,16 @@
 
     os->writeU8(1);
     os->flush();
-  }
 
-  rdr::TLSInStream *tlsis = new rdr::TLSInStream(is, session);
-  rdr::TLSOutStream *tlsos = new rdr::TLSOutStream(os, session);
+    // Create these early as they set up the push/pull functions
+    // for GnuTLS
+    tlsis = new rdr::TLSInStream(is, session);
+    tlsos = new rdr::TLSOutStream(os, session);
+  }
 
   int err;
   err = gnutls_handshake(session);
   if (err != GNUTLS_E_SUCCESS) {
-    delete tlsis;
-    delete tlsos;
-
     if (!gnutls_error_is_fatal(err)) {
       vlog.debug("Deferring completion of TLS handshake: %s", gnutls_strerror(err));
       return false;
@@ -152,7 +155,7 @@
 
   vlog.debug("Handshake completed");
 
-  sc->setStreams(fis = tlsis, fos = tlsos);
+  sc->setStreams(tlsis, tlsos);
 
   return true;
 }