From a64c3e017907712ed23e902d7843a5653a26bba7 Mon Sep 17 00:00:00 2001
From: Michael Jumper <mjumper@apache.org>
Date: Mon, 18 Apr 2016 18:15:32 -0700
Subject: [PATCH] GUACAMOLE-34: Ensure guac_client_stop() or
 guac_client_abort() are called in ALL cases where the client thread
 terminates.

---
 src/protocols/rdp/rdp.c | 19 ++++++++++++++++---
 src/protocols/ssh/ssh.c |  5 ++++-
 src/protocols/vnc/vnc.c | 13 +++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c
index 753312b3..cd3ee231 100644
--- a/src/protocols/rdp/rdp.c
+++ b/src/protocols/rdp/rdp.c
@@ -800,7 +800,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
                 /* Check the libfreerdp fds */
                 if (!freerdp_check_fds(rdp_inst)) {
-                    guac_client_log(client, GUAC_LOG_DEBUG,
+                    guac_client_abort(client,
+                            GUAC_PROTOCOL_STATUS_SERVER_ERROR,
                             "Error handling RDP file descriptors");
                     pthread_mutex_unlock(&(rdp_client->rdp_lock));
                     return 1;
@@ -808,7 +809,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
                 /* Check channel fds */
                 if (!freerdp_channels_check_fds(channels, rdp_inst)) {
-                    guac_client_log(client, GUAC_LOG_DEBUG,
+                    guac_client_abort(client,
+                            GUAC_PROTOCOL_STATUS_SERVER_ERROR,
                             "Error handling RDP channel file descriptors");
                     pthread_mutex_unlock(&(rdp_client->rdp_lock));
                     return 1;
@@ -837,6 +839,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
                 /* Handle RDP disconnect */
                 if (freerdp_shall_disconnect(rdp_inst)) {
+                    guac_client_stop(client);
                     guac_client_log(client, GUAC_LOG_INFO,
                             "RDP server closed connection");
                     pthread_mutex_unlock(&(rdp_client->rdp_lock));
@@ -884,6 +887,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     }
 
+    /* Kill client and finish connection */
+    guac_client_stop(client);
     guac_client_log(client, GUAC_LOG_INFO, "Internal RDP client disconnected");
 
     pthread_mutex_lock(&(rdp_client->rdp_lock));
@@ -953,8 +958,12 @@ void* guac_rdp_client_thread(void* data) {
     if (settings->enable_sftp) {
 
         /* Abort if username is missing */
-        if (settings->sftp_username == NULL)
+        if (settings->sftp_username == NULL) {
+            guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
+                    "A username or SFTP-specific username is required if "
+                    "SFTP is enabled.");
             return NULL;
+        }
 
         guac_client_log(client, GUAC_LOG_DEBUG,
                 "Connecting via SSH for SFTP filesystem access.");
@@ -973,6 +982,8 @@ void* guac_rdp_client_thread(void* data) {
                         settings->sftp_private_key,
                         settings->sftp_passphrase)) {
                 guac_common_ssh_destroy_user(rdp_client->sftp_user);
+                guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
+                        "Private key unreadable.");
                 return NULL;
             }
 
@@ -1015,6 +1026,8 @@ void* guac_rdp_client_thread(void* data) {
         if (rdp_client->sftp_filesystem == NULL) {
             guac_common_ssh_destroy_session(rdp_client->sftp_session);
             guac_common_ssh_destroy_user(rdp_client->sftp_user);
+            guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR,
+                    "SFTP connection failed.");
             return NULL;
         }
 
diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c
index 24494c7d..f887cc23 100644
--- a/src/protocols/ssh/ssh.c
+++ b/src/protocols/ssh/ssh.c
@@ -178,8 +178,11 @@ void* ssh_client_thread(void* data) {
     pthread_t input_thread;
 
     /* Init SSH base libraries */
-    if (guac_common_ssh_init(client))
+    if (guac_common_ssh_init(client)) {
+        guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
+                "SSH library initialization failed");
         return NULL;
+    }
 
     /* Set up screen recording, if requested */
     if (settings->recording_path != NULL) {
diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c
index 5eb6b870..f9ab6873 100644
--- a/src/protocols/vnc/vnc.c
+++ b/src/protocols/vnc/vnc.c
@@ -241,6 +241,13 @@ void* guac_vnc_client_thread(void* data) {
     /* Connect via SSH if SFTP is enabled */
     if (settings->enable_sftp) {
 
+        /* Abort if username is missing */
+        if (settings->sftp_username == NULL) {
+            guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
+                    "SFTP username is required if SFTP is enabled.");
+            return NULL;
+        }
+
         guac_client_log(client, GUAC_LOG_DEBUG,
                 "Connecting via SSH for SFTP filesystem access.");
 
@@ -258,6 +265,8 @@ void* guac_vnc_client_thread(void* data) {
                         settings->sftp_private_key,
                         settings->sftp_passphrase)) {
                 guac_common_ssh_destroy_user(vnc_client->sftp_user);
+                guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,
+                        "Private key unreadable.");
                 return NULL;
             }
 
@@ -297,6 +306,8 @@ void* guac_vnc_client_thread(void* data) {
         if (vnc_client->sftp_filesystem == NULL) {
             guac_common_ssh_destroy_session(vnc_client->sftp_session);
             guac_common_ssh_destroy_user(vnc_client->sftp_user);
+            guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR,
+                    "SFTP connection failed.");
             return NULL;
         }
 
@@ -407,6 +418,8 @@ void* guac_vnc_client_thread(void* data) {
 
     }
 
+    /* Kill client and finish connection */
+    guac_client_stop(client);
     guac_client_log(client, GUAC_LOG_INFO, "Internal VNC client disconnected");
     return NULL;