CVE-2016-10207: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The Xvnc server in TigerVNC allows remote attackers to cause a denial of service (invalid memory access and crash) by terminating a TLS handshake early.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 7.5
Published: 2017-02-28T18:59Z
Repository: https://github.com/TigerVNC/tigervnc
Language: C
Files changed: common/rfb/CSecurityTLS.cxx, common/rfb/CSecurityTLS.h, common/rfb/SSecurityTLS.cxx, common/rfb/SSecurityTLS.h
Commit date: 2016-08-23 17:02:58 +0200
Commit message: Proper global init/deinit of GnuTLS

These are reference counted so it is important to retain symmetry
between the calls. Failure to do so will result in bad memory access
and crashes.

Diff: diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx
index 3dcededb..8a053e3d 100644
--- a/common/rfb/CSecurityTLS.cxx
+++ b/common/rfb/CSecurityTLS.cxx
@@ -65,25 +65,18 @@ using namespace rfb;
 StringParameter CSecurityTLS::X509CA("X509CA", "X509 CA certificate", "", ConfViewer);
 StringParameter CSecurityTLS::X509CRL("X509CRL", "X509 CRL file", "", ConfViewer);
 
 static LogWriter vlog("TLS");
 
-void CSecurityTLS::initGlobal()
-{
-  static bool globalInitDone = false;
-
-  if (!globalInitDone) {
-    gnutls_global_init();
-    globalInitDone = true;
-  }
-}
-
 CSecurityTLS::CSecurityTLS(bool _anon) : session(0), anon_cred(0),
 						 anon(_anon), fis(0), fos(0)
 {
   cafile = X509CA.getData();
   crlfile = X509CRL.getData();
+
+  if (gnutls_global_init() != GNUTLS_E_SUCCESS)
+    throw AuthFailureException("gnutls_global_init failed");
 }
 
 void CSecurityTLS::setDefaults()
 {
   char* homeDir = NULL;
@@ -123,12 +116,10 @@ void CSecurityTLS::shutdown(bool needbye)
   }
 
   if (session) {
     gnutls_deinit(session);
     session = 0;
-
-    gnutls_global_deinit();
   }
 }
 
 
 CSecurityTLS::~CSecurityTLS()
@@ -140,20 +131,20 @@ CSecurityTLS::~CSecurityTLS()
   if (fos)
     delete fos;
 
   delete[] cafile;
   delete[] crlfile;
+
+  gnutls_global_deinit();
 }
 
 bool CSecurityTLS::processMsg(CConnection* cc)
 {
   rdr::InStream* is = cc->getInStream();
   rdr::OutStream* os = cc->getOutStream();
   client = cc;
 
-  initGlobal();
-
   if (!session) {
     if (!is->checkNoWait(1))
       return false;
 
     if (is->readU8() == 0) {
diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h
index b147d802..57d964d7 100644
--- a/common/rfb/CSecurityTLS.h
+++ b/common/rfb/CSecurityTLS.h
@@ -60,12 +60,10 @@ namespace rfb {
     void setParam();
     void checkSession();
     CConnection *client;
 
   private:
-    static void initGlobal();
-
     gnutls_session_t session;
     gnutls_anon_client_credentials_t anon_cred;
     gnutls_certificate_credentials_t cert_cred;
     bool anon;
 
diff --git a/common/rfb/SSecurityTLS.cxx b/common/rfb/SSecurityTLS.cxx
index 0f52d34b..b9460223 100644
--- a/common/rfb/SSecurityTLS.cxx
+++ b/common/rfb/SSecurityTLS.cxx
@@ -46,27 +46,19 @@ StringParameter SSecurityTLS::X509_CertFile
 StringParameter SSecurityTLS::X509_KeyFile
 ("X509Key", "Path to the key of the X509 certificate in PEM format", "", ConfServer);
 
 static LogWriter vlog("TLS");
 
-void SSecurityTLS::initGlobal()
-{
-  static bool globalInitDone = false;
-
-  if (!globalInitDone) {
-    if (gnutls_global_init() != GNUTLS_E_SUCCESS)
-      throw AuthFailureException("gnutls_global_init failed");
-    globalInitDone = true;
-  }
-}
-
 SSecurityTLS::SSecurityTLS(bool _anon) : session(0), dh_params(0),
 						 anon_cred(0), cert_cred(0),
 						 anon(_anon), fis(0), fos(0)
 {
   certfile = X509_CertFile.getData();
   keyfile = X509_KeyFile.getData();
+
+  if (gnutls_global_init() != GNUTLS_E_SUCCESS)
+    throw AuthFailureException("gnutls_global_init failed");
 }
 
 void SSecurityTLS::shutdown()
 {
   if (session) {
@@ -92,12 +84,10 @@ void SSecurityTLS::shutdown()
   }
 
   if (session) {
     gnutls_deinit(session);
     session = 0;
-
-    gnutls_global_deinit();
   }
 }
 
 
 SSecurityTLS::~SSecurityTLS()
@@ -109,22 +99,22 @@ SSecurityTLS::~SSecurityTLS()
   if (fos)
     delete fos;
 
   delete[] keyfile;
   delete[] certfile;
+
+  gnutls_global_deinit();
 }
 
 bool SSecurityTLS::processMsg(SConnection *sc)
 {
   rdr::InStream* is = sc->getInStream();
   rdr::OutStream* os = sc->getOutStream();
 
   vlog.debug("Process security message (session %p)", session);
 
   if (!session) {
-    initGlobal();
-
     if (gnutls_init(&session, GNUTLS_SERVER) != GNUTLS_E_SUCCESS)
       throw AuthFailureException("gnutls_init failed");
 
     if (gnutls_set_default_priority(session) != GNUTLS_E_SUCCESS)
       throw AuthFailureException("gnutls_set_default_priority failed");
diff --git a/common/rfb/SSecurityTLS.h b/common/rfb/SSecurityTLS.h
index a7932054..30242a24 100644
--- a/common/rfb/SSecurityTLS.h
+++ b/common/rfb/SSecurityTLS.h
@@ -52,12 +52,10 @@ namespace rfb {
   protected:
     void shutdown();
     void setParams(gnutls_session_t session);
 
   private:
-    static void initGlobal();
-
     gnutls_session_t session;
     gnutls_dh_params_t dh_params;
     gnutls_anon_server_credentials_t anon_cred;
     gnutls_certificate_credentials_t cert_cred;
     char *keyfile, *certfile;

Vulnerable code: void CSecurityTLS::initGlobal()
{
  static bool globalInitDone = false;

  if (!globalInitDone) {
    gnutls_global_init();
    globalInitDone = true;
  }
}


    gnutls_global_deinit();
  initGlobal();

    static void initGlobal();

void SSecurityTLS::initGlobal()
{
  static bool globalInitDone = false;

  if (!globalInitDone) {
    if (gnutls_global_init() != GNUTLS_E_SUCCESS)
      throw AuthFailureException("gnutls_global_init failed");
    globalInitDone = true;
  }
}


    gnutls_global_deinit();
    initGlobal();

    static void initGlobal();

Fixed code: 
  if (gnutls_global_init() != GNUTLS_E_SUCCESS)
    throw AuthFailureException("gnutls_global_init failed");

  gnutls_global_deinit();

  if (gnutls_global_init() != GNUTLS_E_SUCCESS)
    throw AuthFailureException("gnutls_global_init failed");

  gnutls_global_deinit();