From 944718174d71298522c8dc2c4e5231bc74a04bd9 Mon Sep 17 00:00:00 2001 From: Mike Jumper Date: Sun, 15 Oct 2023 21:12:03 -0700 Subject: [PATCH] GUACAMOLE-1867: Migrate SSH to new memory management functions. --- src/common-ssh/buffer.c | 6 ++- src/common-ssh/common-ssh/ssh.h | 2 +- src/common-ssh/key.c | 14 +++---- src/common-ssh/sftp.c | 21 +++++----- src/common-ssh/ssh.c | 22 ++++++----- src/common-ssh/tests/sftp/normalize_path.c | 24 ++++++------ src/common-ssh/user.c | 17 ++++---- src/protocols/ssh/client.c | 5 ++- src/protocols/ssh/settings.c | 45 +++++++++++----------- src/protocols/ssh/ssh.c | 3 +- src/protocols/ssh/ssh_agent.c | 3 +- 11 files changed, 88 insertions(+), 74 deletions(-) diff --git a/src/common-ssh/buffer.c b/src/common-ssh/buffer.c index aa79bafa..b7763f69 100644 --- a/src/common-ssh/buffer.c +++ b/src/common-ssh/buffer.c @@ -22,6 +22,8 @@ #include #include +#include + #include #include #include @@ -67,7 +69,7 @@ void guac_common_ssh_buffer_write_bignum(char** buffer, const BIGNUM* value) { /* Allocate output buffer, add padding byte */ length = BN_num_bytes(value); - bn_buffer = malloc(length); + bn_buffer = guac_mem_alloc(length); /* Convert BIGNUM */ BN_bn2bin(value, bn_buffer); @@ -84,7 +86,7 @@ void guac_common_ssh_buffer_write_bignum(char** buffer, const BIGNUM* value) { memcpy(*buffer, bn_buffer, length); *buffer += length; - free(bn_buffer); + guac_mem_free(bn_buffer); } diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 346d8abc..986c63b2 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -38,7 +38,7 @@ * * @return * A newly-allocated string containing the credentials provided by - * the user, which must be freed by a call to free(). + * the user, which must be freed by a call to guac_mem_free(). */ typedef char* guac_ssh_credential_handler(guac_client* client, char* cred_name); diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index cecfb413..ddc84179 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -22,6 +22,7 @@ #include "common-ssh/buffer.h" #include "common-ssh/key.h" +#include #include #include @@ -137,13 +138,13 @@ guac_common_ssh_key* guac_common_ssh_key_alloc(char* data, int length, if (is_passphrase_needed(data, length) && (passphrase == NULL || *passphrase == '\0')) return NULL; - guac_common_ssh_key* key = malloc(sizeof(guac_common_ssh_key)); + guac_common_ssh_key* key = guac_mem_alloc(sizeof(guac_common_ssh_key)); /* Copy private key to structure */ key->private_key_length = length; - key->private_key = malloc(length); + key->private_key = guac_mem_alloc(length); memcpy(key->private_key, data, length); - key->passphrase = strdup(passphrase); + key->passphrase = guac_strdup(passphrase); return key; @@ -157,10 +158,9 @@ const char* guac_common_ssh_key_error() { } void guac_common_ssh_key_free(guac_common_ssh_key* key) { - - free(key->private_key); - free(key->passphrase); - free(key); + guac_mem_free(key->private_key); + guac_mem_free(key->passphrase); + guac_mem_free(key); } int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* client, diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 0100cc16..4a6f7716 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -21,6 +21,7 @@ #include "common-ssh/ssh.h" #include +#include #include #include #include @@ -622,7 +623,7 @@ static int guac_common_ssh_sftp_ls_ack_handler(guac_user* user, if (status != GUAC_PROTOCOL_STATUS_SUCCESS) { libssh2_sftp_closedir(list_state->directory); guac_user_free_stream(user, stream); - free(list_state); + guac_mem_free(list_state); return 0; } @@ -674,7 +675,7 @@ static int guac_common_ssh_sftp_ls_ack_handler(guac_user* user, /* Clean up resources */ libssh2_sftp_closedir(list_state->directory); - free(list_state); + guac_mem_free(list_state); /* Signal of stream */ guac_protocol_send_end(user->socket, stream); @@ -774,7 +775,7 @@ static int guac_common_ssh_sftp_get_handler(guac_user* user, /* Init directory listing state */ guac_common_ssh_sftp_ls_state* list_state = - malloc(sizeof(guac_common_ssh_sftp_ls_state)); + guac_mem_alloc(sizeof(guac_common_ssh_sftp_ls_state)); list_state->directory = dir; list_state->filesystem = filesystem; @@ -786,7 +787,7 @@ static int guac_common_ssh_sftp_get_handler(guac_user* user, if (length >= sizeof(list_state->directory_name)) { guac_user_log(user, GUAC_LOG_INFO, "Unable to read directory " "\"%s\": Path too long", fullpath); - free(list_state); + guac_mem_free(list_state); return 0; } @@ -969,7 +970,7 @@ guac_common_ssh_sftp_filesystem* guac_common_ssh_create_sftp_filesystem( /* Allocate data for SFTP session */ guac_common_ssh_sftp_filesystem* filesystem = - malloc(sizeof(guac_common_ssh_sftp_filesystem)); + guac_mem_alloc(sizeof(guac_common_ssh_sftp_filesystem)); /* Associate SSH session with SFTP data and user */ filesystem->ssh_session = session; @@ -984,15 +985,15 @@ guac_common_ssh_sftp_filesystem* guac_common_ssh_create_sftp_filesystem( root_path)) { guac_client_log(session->client, GUAC_LOG_WARNING, "Cannot create " "SFTP filesystem - \"%s\" is not a valid path.", root_path); - free(filesystem); + guac_mem_free(filesystem); return NULL; } /* Generate filesystem name from root path if no name is provided */ if (name != NULL) - filesystem->name = strdup(name); + filesystem->name = guac_strdup(name); else - filesystem->name = strdup(filesystem->root_path); + filesystem->name = guac_strdup(filesystem->root_path); /* Initially upload files to current directory */ strcpy(filesystem->upload_path, "."); @@ -1009,8 +1010,8 @@ void guac_common_ssh_destroy_sftp_filesystem( libssh2_sftp_shutdown(filesystem->sftp_session); /* Free associated memory */ - free(filesystem->name); - free(filesystem); + guac_mem_free(filesystem->name); + guac_mem_free(filesystem); } diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 373b9f12..c4c3b4cc 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -23,6 +23,8 @@ #include #include +#include +#include #include #ifdef LIBSSH2_USES_GCRYPT @@ -120,7 +122,7 @@ static void guac_common_ssh_openssl_init_locks(int count) { /* Allocate required number of locks */ guac_common_ssh_openssl_locks = - malloc(sizeof(pthread_mutex_t) * count); + guac_mem_alloc(sizeof(pthread_mutex_t), count); /* Initialize each lock */ for (i=0; i < count; i++) @@ -147,7 +149,7 @@ static void guac_common_ssh_openssl_free_locks(int count) { pthread_mutex_destroy(&(guac_common_ssh_openssl_locks[i])); /* Free lock array */ - free(guac_common_ssh_openssl_locks); + guac_mem_free(guac_common_ssh_openssl_locks); } #endif @@ -249,7 +251,7 @@ static void guac_common_ssh_kbd_callback(const char *name, int name_len, /* Send password if only one prompt */ if (num_prompts == 1) { char* password = common_session->user->password; - responses[0].text = strdup(password); + responses[0].text = guac_strdup(password); responses[0].length = strlen(password); } @@ -486,7 +488,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* Allocate new session */ guac_common_ssh_session* common_session = - malloc(sizeof(guac_common_ssh_session)); + guac_mem_alloc(sizeof(guac_common_ssh_session)); /* Open SSH session */ LIBSSH2_SESSION* session = libssh2_session_init_ex(NULL, NULL, @@ -494,7 +496,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (session == NULL) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Session allocation failed."); - free(common_session); + guac_mem_free(common_session); close(fd); return NULL; } @@ -514,7 +516,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (libssh2_session_handshake(session, fd)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "SSH handshake failed."); - free(common_session); + guac_mem_free(common_session); close(fd); return NULL; } @@ -527,7 +529,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (!remote_hostkey) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Failed to get host key for %s", hostname); - free(common_session); + guac_mem_free(common_session); close(fd); return NULL; } @@ -550,7 +552,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Host key did not match any provided known host keys. %s", err_msg); - free(common_session); + guac_mem_free(common_session); close(fd); return NULL; } @@ -564,7 +566,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* Attempt authentication */ if (guac_common_ssh_authenticate(common_session)) { - free(common_session); + guac_mem_free(common_session); close(fd); return NULL; } @@ -595,6 +597,6 @@ void guac_common_ssh_destroy_session(guac_common_ssh_session* session) { libssh2_session_free(session->session); /* Free all other data */ - free(session); + guac_mem_free(session); } diff --git a/src/common-ssh/tests/sftp/normalize_path.c b/src/common-ssh/tests/sftp/normalize_path.c index 151b1835..df586c10 100644 --- a/src/common-ssh/tests/sftp/normalize_path.c +++ b/src/common-ssh/tests/sftp/normalize_path.c @@ -19,6 +19,8 @@ #include "common-ssh/sftp.h" +#include + #include #include @@ -160,7 +162,7 @@ void test_sftp__normalize_relative_mixed() { * Generates a dynamically-allocated path having the given number of bytes, not * counting the null-terminator. The path will contain only UNIX-style path * separators. The returned path must eventually be freed with a call to - * free(). + * guac_mem_free(). * * @param length * The number of bytes to include in the generated path, not counting the @@ -174,16 +176,16 @@ void test_sftp__normalize_relative_mixed() { * @return * A dynamically-allocated path containing the given number of bytes, not * counting the null-terminator. This path must eventually be freed with a - * call to free(). + * call to guac_mem_free(). */ static char* generate_path(int length, int max_depth) { /* If no length given, calculate space required from max_depth */ if (length == -1) - length = max_depth * 2; + length = guac_mem_ckd_mul_or_die(max_depth, 2); int i; - char* input = malloc(length + 1); + char* input = guac_mem_alloc(guac_mem_ckd_add_or_die(length, 1)); /* Fill path with /x/x/x/x/x/x/x/x/x/x/.../xxxxxxxxx... */ for (i = 0; i < length; i++) { @@ -214,17 +216,17 @@ void test_sftp__normalize_long() { /* Exceeds maximum length by a factor of 2 */ input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH * 2, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); /* Exceeds maximum length by one byte */ input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); /* Exactly maximum length */ input = generate_path(GUAC_COMMON_SSH_SFTP_MAX_PATH - 1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); } @@ -240,24 +242,24 @@ void test_sftp__normalize_deep() { /* Exceeds maximum depth by a factor of 2 */ input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH * 2); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); /* Exceeds maximum depth by one component */ input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH + 1); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); /* Exactly maximum depth (should still be rejected as SFTP depth limits are * set such that a path with the maximum depth will exceed the maximum * length) */ input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH); CU_ASSERT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); /* Less than maximum depth */ input = generate_path(-1, GUAC_COMMON_SSH_SFTP_MAX_DEPTH - 1); CU_ASSERT_NOT_EQUAL(guac_common_ssh_sftp_normalize_path(normalized, input), 0); - free(input); + guac_mem_free(input); } diff --git a/src/common-ssh/user.c b/src/common-ssh/user.c index 92c8d963..24c74dce 100644 --- a/src/common-ssh/user.c +++ b/src/common-ssh/user.c @@ -20,15 +20,18 @@ #include "common-ssh/key.h" #include "common-ssh/user.h" +#include +#include + #include #include guac_common_ssh_user* guac_common_ssh_create_user(const char* username) { - guac_common_ssh_user* user = malloc(sizeof(guac_common_ssh_user)); + guac_common_ssh_user* user = guac_mem_alloc(sizeof(guac_common_ssh_user)); /* Init user */ - user->username = strdup(username); + user->username = guac_strdup(username); user->password = NULL; user->private_key = NULL; @@ -43,9 +46,9 @@ void guac_common_ssh_destroy_user(guac_common_ssh_user* user) { guac_common_ssh_key_free(user->private_key); /* Free all other data */ - free(user->password); - free(user->username); - free(user); + guac_mem_free(user->password); + guac_mem_free(user->username); + guac_mem_free(user); } @@ -53,8 +56,8 @@ void guac_common_ssh_user_set_password(guac_common_ssh_user* user, const char* password) { /* Replace current password with given value */ - free(user->password); - user->password = strdup(password); + guac_mem_free(user->password); + user->password = guac_strdup(password); } diff --git a/src/protocols/ssh/client.c b/src/protocols/ssh/client.c index c1ef76ff..b116e6d1 100644 --- a/src/protocols/ssh/client.c +++ b/src/protocols/ssh/client.c @@ -33,6 +33,7 @@ #include #include +#include #include #include @@ -69,7 +70,7 @@ int guac_client_init(guac_client* client) { client->args = GUAC_SSH_CLIENT_ARGS; /* Allocate client instance data */ - guac_ssh_client* ssh_client = calloc(1, sizeof(guac_ssh_client)); + guac_ssh_client* ssh_client = guac_mem_zalloc(sizeof(guac_ssh_client)); client->data = ssh_client; /* Set handlers */ @@ -143,7 +144,7 @@ int guac_ssh_client_free_handler(guac_client* client) { guac_ssh_settings_free(ssh_client->settings); /* Free client structure */ - free(ssh_client); + guac_mem_free(ssh_client); guac_common_ssh_uninit(); return 0; diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 686ef637..ea890cec 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -25,6 +25,7 @@ #include "settings.h" #include "terminal/terminal.h" +#include #include #include @@ -345,7 +346,7 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, return NULL; } - guac_ssh_settings* settings = calloc(1, sizeof(guac_ssh_settings)); + guac_ssh_settings* settings = guac_mem_zalloc(sizeof(guac_ssh_settings)); /* Read parameters */ settings->hostname = @@ -558,48 +559,48 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, void guac_ssh_settings_free(guac_ssh_settings* settings) { /* Free network connection information */ - free(settings->hostname); - free(settings->host_key); - free(settings->port); + guac_mem_free(settings->hostname); + guac_mem_free(settings->host_key); + guac_mem_free(settings->port); /* Free credentials */ - free(settings->username); - free(settings->password); - free(settings->key_base64); - free(settings->key_passphrase); + guac_mem_free(settings->username); + guac_mem_free(settings->password); + guac_mem_free(settings->key_base64); + guac_mem_free(settings->key_passphrase); /* Free display preferences */ - free(settings->font_name); - free(settings->color_scheme); + guac_mem_free(settings->font_name); + guac_mem_free(settings->color_scheme); /* Free requested command */ - free(settings->command); + guac_mem_free(settings->command); /* Free SFTP settings */ - free(settings->sftp_root_directory); + guac_mem_free(settings->sftp_root_directory); /* Free typescript settings */ - free(settings->typescript_name); - free(settings->typescript_path); + guac_mem_free(settings->typescript_name); + guac_mem_free(settings->typescript_path); /* Free screen recording settings */ - free(settings->recording_name); - free(settings->recording_path); + guac_mem_free(settings->recording_name); + guac_mem_free(settings->recording_path); /* Free terminal emulator type. */ - free(settings->terminal_type); + guac_mem_free(settings->terminal_type); /* Free locale */ - free(settings->locale); + guac_mem_free(settings->locale); /* Free the client timezone. */ - free(settings->timezone); + guac_mem_free(settings->timezone); /* Free Wake-on-LAN settings. */ - free(settings->wol_mac_addr); - free(settings->wol_broadcast_addr); + guac_mem_free(settings->wol_mac_addr); + guac_mem_free(settings->wol_broadcast_addr); /* Free overall structure */ - free(settings); + guac_mem_free(settings); } diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 2ebe0687..05255761 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -256,7 +257,7 @@ void* ssh_client_thread(void* data) { ssh_client->term = guac_terminal_create(client, options); /* Free options struct now that it's been used */ - free(options); + guac_mem_free(options); /* Fail if terminal init failed */ if (ssh_client->term == NULL) { diff --git a/src/protocols/ssh/ssh_agent.c b/src/protocols/ssh/ssh_agent.c index 199c3b58..0afbae09 100644 --- a/src/protocols/ssh/ssh_agent.c +++ b/src/protocols/ssh/ssh_agent.c @@ -24,6 +24,7 @@ #include "ssh_buffer.h" #include +#include #include #include #include @@ -189,7 +190,7 @@ void ssh_auth_agent_callback(LIBSSH2_SESSION *session, ssh_guac_client_data* client_data = (ssh_guac_client_data*) client->data; /* Init auth agent */ - ssh_auth_agent* auth_agent = malloc(sizeof(ssh_auth_agent)); + ssh_auth_agent* auth_agent = guac_mem_alloc(sizeof(ssh_auth_agent)); auth_agent->channel = channel; auth_agent->identity = client_data->key; auth_agent->buffer_length = 0;