diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h index 0f20674ba3..4c0627c868 100644 --- a/src/libguac/guacamole/string.h +++ b/src/libguac/guacamole/string.h @@ -26,9 +26,39 @@ * @file string.h */ +#include #include #include +/** + * String buffer size needed to represent 'int', and 'unsigned short'in decimal + * format. This includes the NULL terminator. + * + * Integer size can vary by platform, but short size is consistent across + * all modern platforms. + */ +#if (__SIZEOF_INT__ == 2) /* INT_MIN: '-32768' */ + #define GUAC_INT_STRING_BUFSIZE 7 +#elif (__SIZEOF_INT__ == 4) /* INT_MIN: '-2147483648' */ + #define GUAC_INT_STRING_BUFSIZE 12 +#elif (__SIZEOF_INT__ == 8) /* INT_MIN: '-9223372036854775808' */ + #define GUAC_INT_STRING_BUFSIZE 21 +#else + #error Unsupported unsigned int size +#endif + +#define GUAC_USHORT_STRING_BUFSIZE 6 /* '65535' */ + +/** + * Convenience macros for the minimum and maximum values for int and and + * unsigned short data types. + */ +#define GUAC_ITOA_INT_MIN INT_MIN +#define GUAC_ITOA_INT_MAX INT_MAX + +#define GUAC_ITOA_USHORT_MAX USHRT_MAX +#define GUAC_ITOA_USHORT_MIN 0 + /** * Convert the provided unsigned integer into a string, returning the number of * characters written into the destination string, or a negative value if an @@ -37,15 +67,35 @@ * @param dest * The destination string to copy the data into, which should already be * allocated and at a size that can handle the string representation of the - * inteer. + * integer (at least GUAC_INT_STRING_BUFSIZE bytes). * * @param integer - * The unsigned integer to convert to a string. + * The integer to convert to a string. * * @return * The number of characters written into the dest string. */ -int guac_itoa(char* restrict dest, unsigned int integer); +int guac_itoa(char* restrict dest, int integer); + +/** + * Converts the given integer to a string safely. The resulting string will be + * written into the provided buffer, ensuring that the buffer is not exceeded. + * The conversion will fail if the buffer is too small to hold the result. + * + * @param dest + * The buffer to write the converted string into. + * + * @param dest_size + * The size of the provided buffer, in bytes. + * + * @param integer + * The integer to convert to a string. + * + * @return + * The number of characters written (excluding the null terminator), or + * -1 if the string was truncated, or a negative value if an error occurs. + */ +int guac_itoa_safe(char* restrict dest, size_t dest_size, int integer); /** * Copies a limited number of bytes from the given source string to the given diff --git a/src/libguac/guacamole/user.h b/src/libguac/guacamole/user.h index 13fb146d4d..93e68ea0d1 100644 --- a/src/libguac/guacamole/user.h +++ b/src/libguac/guacamole/user.h @@ -932,6 +932,98 @@ int guac_user_supports_webp(guac_user* user); char* guac_user_parse_args_string(guac_user* user, const char** arg_names, const char** argv, int index, const char* default_value); +/** + * Automatically handles a single argument received from a joining user, + * returning a newly-allocated string containing that value. The argument + * is parsed as an integer and checked to ensure it is within the specified + * bounds. If the argument provided by the user is blank, a newly-allocated + * string containing the default value is returned. If the integer is out of + * bounds, the default value is returned. + * + * @param user + * The user joining the connection and providing the given arguments. + * + * @param arg_names + * A NULL-terminated array of argument names, corresponding to the provided + * array of argument values. This array must be exactly the same size as + * the argument value array, with one additional entry for the NULL + * terminator. + * + * @param argv + * An array of all argument values, corresponding to the provided array of + * argument names. This array must be exactly the same size as the argument + * name array, with the exception of the NULL terminator. + * + * @param index + * The index of the entry in both the arg_names and argv arrays which + * corresponds to the argument being parsed. + * + * @param default_value + * The value to return if the provided argument is blank or invalid. If this + * value is not NULL, the returned value will be a newly-allocated string + * containing this value. + * + * @param min + * The minimum allowed value for the parsed integer. If the parsed integer is + * less than this value, the default value will be returned. + * + * @param max + * The maximum allowed value for the parsed integer. If the parsed integer is + * greater than this value, the default value will be returned. + * + * @return + * A newly-allocated string containing the provided argument value, parsed as + * an integer and checked against the specified bounds. If the argument is blank, + * or if the integer is out of bounds, the function returns a newly-allocated string + * containing the default value. If the default value is NULL and the provided + * argument is blank or invalid, NULL is returned. + */ +char* guac_user_parse_args_int_string_bounded(guac_user* user, const char** arg_names, + const char** argv, int index, const char* default_value, int min, int max); + +/** + * Automatically handles a single integer argument received from a joining + * user, returning the integer value of that argument. The argument is checked + * to ensure it is within the specified bounds. If the argument is blank, + * invalid, or out of bounds, the default value is returned. + * + * @param user + * The user joining the connection and providing the given arguments. + * + * @param arg_names + * A NULL-terminated array of argument names, corresponding to the provided + * array of argument values. This array must be exactly the same size as + * the argument value array, with one additional entry for the NULL + * terminator. + * + * @param argv + * An array of all argument values, corresponding to the provided array of + * argument names. This array must be exactly the same size as the argument + * name array, with the exception of the NULL terminator. + * + * @param index + * The index of the entry in both the arg_names and argv arrays which + * corresponds to the argument being parsed. + * + * @param default_value + * The value to return if the provided argument is blank, invalid, or out of bounds. + * + * @param min + * The minimum allowed value for the argument. If the parsed value is less + * than this, the default value will be returned. + * + * @param max + * The maximum allowed value for the argument. If the parsed value is greater + * than this, the default value will be returned. + * + * @return + * The integer value parsed from the provided argument value, if it is valid + * and within the specified bounds. If the argument is blank, invalid, or out of bounds, + * the default value is returned. + */ +int guac_user_parse_args_int_bounded(guac_user* user, const char** arg_names, + const char** argv, int index, int default_value, int min, int max); + /** * Automatically handles a single integer argument received from a joining * user, returning the integer value of that argument. If the argument provided diff --git a/src/libguac/guacamole/wol-constants.h b/src/libguac/guacamole/wol-constants.h index 831a475e3e..38bcf5deb6 100644 --- a/src/libguac/guacamole/wol-constants.h +++ b/src/libguac/guacamole/wol-constants.h @@ -20,6 +20,8 @@ #ifndef GUAC_WOL_CONSTANTS_H #define GUAC_WOL_CONSTANTS_H +#include "guacamole/string.h" + /** * Header file that provides constants and defaults related to libguac * Wake-on-LAN support. diff --git a/src/libguac/string.c b/src/libguac/string.c index a2a6e74c55..7793474112 100644 --- a/src/libguac/string.c +++ b/src/libguac/string.c @@ -59,6 +59,21 @@ int guac_itoa(char* restrict dest, unsigned int integer) { } +int guac_itoa_safe(char* restrict dest, size_t dest_size, int integer) +{ + int str_size = snprintf(dest, dest_size, "%d", integer); + + if (str_size < 0) + return str_size; + + /* Return an error if the string was truncated. */ + if ((size_t)str_size >= dest_size) + return -1; + + /* Return the number of characters written (excluding the terminator). */ + return str_size; +} + size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n) { #ifdef HAVE_STRLCPY diff --git a/src/libguac/user.c b/src/libguac/user.c index 753aca3fae..62220555d7 100644 --- a/src/libguac/user.c +++ b/src/libguac/user.c @@ -366,12 +366,15 @@ char* guac_user_parse_args_string(guac_user* user, const char** arg_names, /* Pull parameter value from argv */ const char* value = argv[index]; - /* Use default value if blank */ - if (value[0] == 0) { + /* Use default_value if value is NULL, or an empty string */ + if (value == NULL || value[0] == 0) { /* NULL is a completely legal default value */ - if (default_value == NULL) + if (default_value == NULL) { + guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. No default value provided.", + arg_names[index]); return NULL; + } /* Log use of default */ guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. Using " @@ -386,8 +389,65 @@ char* guac_user_parse_args_string(guac_user* user, const char** arg_names, } -int guac_user_parse_args_int(guac_user* user, const char** arg_names, - const char** argv, int index, int default_value) { +char* guac_user_parse_args_int_string_bounded(guac_user* user, const char** arg_names, + const char** argv, int index, const char* default_value, int min, int max) { + + char* parse_end; + long parsed_value; + + /* Pull parameter value from argv */ + const char* value = argv[index]; + + /* Use default_value if value is NULL, or an empty string */ + if (value == NULL || value[0] == 0) { + + /* NULL is a completely legal default value */ + if (default_value == NULL) { + guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. No default value provided.", + arg_names[index]); + + return NULL; + } + + /* Log use of default */ + guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. Using " + "default value of \"%s\".", arg_names[index], default_value); + + return guac_strdup(default_value); + + } + + /* Parse value using strtol, checking for errors */ + errno = 0; + parsed_value = strtol(value, &parse_end, 10); + + /* Check for parsing errors or invalid range */ + if (errno != 0 || *parse_end != '\0' || parsed_value < min || parsed_value > max) { + + /* NULL is a completely legal default value */ + if (default_value == NULL) { + guac_user_log(user, GUAC_LOG_WARNING, "Specified value \"%s\" for " + "parameter \"%s\" is not valid. No default value provided", + value, arg_names[index]); + + return NULL; + } + + /* Log use of default */ + guac_user_log(user, GUAC_LOG_WARNING, "Specified value \"%s\" for " + "parameter \"%s\" is not valid. Using default value " + "of \"%s\".", value, arg_names[index], default_value); + + return guac_strdup(default_value); + + } + + /* Otherwise use provided value */ + return guac_strdup(value); +} + +int guac_user_parse_args_int_bounded(guac_user* user, const char** arg_names, + const char** argv, int index, int default_value, int min, int max) { char* parse_end; long parsed_value; @@ -395,8 +455,8 @@ int guac_user_parse_args_int(guac_user* user, const char** arg_names, /* Pull parameter value from argv */ const char* value = argv[index]; - /* Use default value if blank */ - if (value[0] == 0) { + /* Use default_value if value is NULL, or an empty string */ + if (value == NULL || value[0] == 0) { /* Log use of default */ guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. Using " @@ -410,16 +470,12 @@ int guac_user_parse_args_int(guac_user* user, const char** arg_names, errno = 0; parsed_value = strtol(value, &parse_end, 10); - /* Ensure parsed value is within the legal range of an int */ - if (parsed_value < INT_MIN || parsed_value > INT_MAX) - errno = ERANGE; - - /* Resort to default if input is invalid */ - if (errno != 0 || *parse_end != '\0') { + /* Check for parsing errors or invalid range */ + if (errno != 0 || *parse_end != '\0' || parsed_value < min || parsed_value > max) { /* Log use of default */ guac_user_log(user, GUAC_LOG_WARNING, "Specified value \"%s\" for " - "parameter \"%s\" is not a valid integer. Using default value " + "parameter \"%s\" is not valid. Using default value " "of %i.", value, arg_names[index], default_value); return default_value; @@ -427,7 +483,15 @@ int guac_user_parse_args_int(guac_user* user, const char** arg_names, } /* Parsed successfully */ - return parsed_value; + return (int)parsed_value; + +} + +int guac_user_parse_args_int(guac_user* user, const char** arg_names, + const char** argv, int index, int default_value) { + + return guac_user_parse_args_int_bounded(user, arg_names, argv, index, + default_value, GUAC_ITOA_INT_MIN, GUAC_ITOA_INT_MAX); } @@ -437,8 +501,8 @@ int guac_user_parse_args_boolean(guac_user* user, const char** arg_names, /* Pull parameter value from argv */ const char* value = argv[index]; - /* Use default value if blank */ - if (value[0] == 0) { + /* Use default_value if value is NULL, or an empty string */ + if (value == NULL || value[0] == 0) { /* Log use of default */ guac_user_log(user, GUAC_LOG_DEBUG, "Parameter \"%s\" omitted. Using " diff --git a/src/protocols/kubernetes/settings.c b/src/protocols/kubernetes/settings.c index 698fcc1eda..240c0e9d07 100644 --- a/src/protocols/kubernetes/settings.c +++ b/src/protocols/kubernetes/settings.c @@ -278,9 +278,9 @@ guac_kubernetes_settings* guac_kubernetes_parse_args(guac_user* user, IDX_HOSTNAME, ""); /* Read port */ - settings->port = - guac_user_parse_args_int(user, GUAC_KUBERNETES_CLIENT_ARGS, argv, - IDX_PORT, GUAC_KUBERNETES_DEFAULT_PORT); + settings->port = (unsigned short) + guac_user_parse_args_int_bounded(user, GUAC_KUBERNETES_CLIENT_ARGS, argv, + IDX_PORT, GUAC_KUBERNETES_DEFAULT_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Read Kubernetes namespace */ settings->kubernetes_namespace = diff --git a/src/protocols/kubernetes/settings.h b/src/protocols/kubernetes/settings.h index 468029c939..2ea5f5346e 100644 --- a/src/protocols/kubernetes/settings.h +++ b/src/protocols/kubernetes/settings.h @@ -61,7 +61,7 @@ typedef struct guac_kubernetes_settings { /** * The port of the Kubernetes server to connect to. */ - int port; + unsigned short port; /** * The name of the Kubernetes namespace of the pod containing the container diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 669bc82ee0..2b62896a32 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -737,13 +737,12 @@ void* guac_rdp_client_thread(void* data) { */ if (settings->wol_wait_time > 0) { guac_client_log(client, GUAC_LOG_DEBUG, "Sending Wake-on-LAN packet, " - "and pausing for %d seconds.", settings->wol_wait_time); + "and retrying connection check %d times every %d seconds.", + GUAC_WOL_DEFAULT_CONNECT_RETRIES, settings->wol_wait_time); - /* char representation of a port should be, at most, 5 digits plus terminator. */ - char* str_port = guac_mem_alloc(6); - if (guac_itoa(str_port, settings->port) < 1) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to convert port to integer for WOL function."); - guac_mem_free(str_port); + char str_port[GUAC_USHORT_STRING_BUFSIZE]; + if (guac_itoa_safe(str_port, sizeof(str_port), settings->port) < 1) { + guac_client_log(client, GUAC_LOG_ERROR, "Failed to convert port to string for WOL function."); return NULL; } @@ -754,26 +753,27 @@ void* guac_rdp_client_thread(void* data) { settings->wol_wait_time, GUAC_WOL_DEFAULT_CONNECT_RETRIES, settings->hostname, - (const char *) str_port, - GUAC_WOL_DEFAULT_CONNECTION_TIMEOUT)) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet, or server failed to wake up."); - guac_mem_free(str_port); + str_port, + settings->timeout)) { + guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet, or connect to remote server."); return NULL; } - - guac_mem_free(str_port); - } - /* Just send the packet and continue the connection, or return if failed. */ - else if(guac_wol_wake(settings->wol_mac_addr, + /* Just send the packet, or return NULL if failed. */ + else { + guac_client_log(client, GUAC_LOG_DEBUG, "Sending RDP Wake-on-LAN packet."); + + if(guac_wol_wake(settings->wol_mac_addr, settings->wol_broadcast_addr, settings->wol_udp_port)) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet."); - return NULL; + guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet."); + return NULL; + } } + } - + /* If audio enabled, choose an encoder */ if (settings->audio_enabled) { diff --git a/src/protocols/rdp/settings.c b/src/protocols/rdp/settings.c index 317c443b86..f94b731f9c 100644 --- a/src/protocols/rdp/settings.c +++ b/src/protocols/rdp/settings.c @@ -832,9 +832,10 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, IDX_HOSTNAME, ""); /* If port specified, use it, otherwise use an appropriate default */ - settings->port = - guac_user_parse_args_int(user, GUAC_RDP_CLIENT_ARGS, argv, IDX_PORT, - settings->security_mode == GUAC_SECURITY_VMCONNECT ? RDP_DEFAULT_VMCONNECT_PORT : RDP_DEFAULT_PORT); + settings->port = (unsigned short) + guac_user_parse_args_int_bounded(user, GUAC_RDP_CLIENT_ARGS, argv, IDX_PORT, + settings->security_mode == GUAC_SECURITY_VMCONNECT ? RDP_DEFAULT_VMCONNECT_PORT : RDP_DEFAULT_PORT, + GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Look for timeout settings and parse or set defaults. */ settings->timeout = @@ -1109,8 +1110,8 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, /* Port for SFTP connection */ settings->sftp_port = - guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, - IDX_SFTP_PORT, "22"); + guac_user_parse_args_int_string_bounded(user, GUAC_RDP_CLIENT_ARGS, argv, + IDX_SFTP_PORT, "22", GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* SFTP timeout */ settings->sftp_timeout = @@ -1260,9 +1261,9 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, IDX_GATEWAY_HOSTNAME, NULL); /* If gateway port specified, use it */ - settings->gateway_port = - guac_user_parse_args_int(user, GUAC_RDP_CLIENT_ARGS, argv, - IDX_GATEWAY_PORT, 443); + settings->gateway_port = (unsigned short) + guac_user_parse_args_int_bounded(user, GUAC_RDP_CLIENT_ARGS, argv, + IDX_GATEWAY_PORT, 443, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Set gateway domain */ settings->gateway_domain = @@ -1348,8 +1349,8 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, /* Parse the WoL broadcast port. */ settings->wol_udp_port = (unsigned short) - guac_user_parse_args_int(user, GUAC_RDP_CLIENT_ARGS, argv, - IDX_WOL_UDP_PORT, GUAC_WOL_PORT); + guac_user_parse_args_int_bounded(user, GUAC_RDP_CLIENT_ARGS, argv, + IDX_WOL_UDP_PORT, GUAC_WOL_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Parse the WoL wait time. */ settings->wol_wait_time = diff --git a/src/protocols/rdp/settings.h b/src/protocols/rdp/settings.h index 253b7628bb..db9ad70a37 100644 --- a/src/protocols/rdp/settings.h +++ b/src/protocols/rdp/settings.h @@ -164,7 +164,7 @@ typedef struct guac_rdp_settings { /** * The port to connect to. */ - int port; + unsigned short port; /** * The timeout, in seconds, to wait for the remote host to respond. @@ -624,7 +624,7 @@ typedef struct guac_rdp_settings { * FreeRDP prior to 1.2 which have gateway support ignore this value, and * instead use a hard-coded value of 443. */ - int gateway_port; + unsigned short gateway_port; /** * The domain of the user authenticating with the remote desktop gateway, diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 28abb0c56b..099b120054 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -26,9 +26,11 @@ #include "terminal/terminal.h" #include +#include #include #include +#include #include #include #include @@ -457,8 +459,8 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, /* Read port */ settings->port = - guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, - IDX_PORT, GUAC_SSH_DEFAULT_PORT); + guac_user_parse_args_int_string_bounded(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_PORT, GUAC_SSH_DEFAULT_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Parse the timeout value. */ settings->timeout = @@ -587,8 +589,8 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, IDX_WOL_BROADCAST_ADDR, GUAC_WOL_LOCAL_IPV4_BROADCAST); settings->wol_udp_port = (unsigned short) - guac_user_parse_args_int(user, GUAC_SSH_CLIENT_ARGS, argv, - IDX_WOL_UDP_PORT, GUAC_WOL_PORT); + guac_user_parse_args_int_bounded(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_WOL_UDP_PORT, GUAC_WOL_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); settings->wol_wait_time = guac_user_parse_args_int(user, GUAC_SSH_CLIENT_ARGS, argv, diff --git a/src/protocols/telnet/settings.c b/src/protocols/telnet/settings.c index 44a143e686..264af339f2 100644 --- a/src/protocols/telnet/settings.c +++ b/src/protocols/telnet/settings.c @@ -25,10 +25,12 @@ #include "terminal/terminal.h" #include +#include #include #include #include +#include #include #include #include @@ -447,8 +449,8 @@ guac_telnet_settings* guac_telnet_parse_args(guac_user* user, /* Read port */ settings->port = - guac_user_parse_args_string(user, GUAC_TELNET_CLIENT_ARGS, argv, - IDX_PORT, GUAC_TELNET_DEFAULT_PORT); + guac_user_parse_args_int_string_bounded(user, GUAC_TELNET_CLIENT_ARGS, argv, + IDX_PORT, GUAC_TELNET_DEFAULT_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Read connection timeout */ settings->timeout = @@ -556,8 +558,8 @@ guac_telnet_settings* guac_telnet_parse_args(guac_user* user, /* Parse the WoL broadcast port. */ settings->wol_udp_port = (unsigned short) - guac_user_parse_args_int(user, GUAC_TELNET_CLIENT_ARGS, argv, - IDX_WOL_UDP_PORT, GUAC_WOL_PORT); + guac_user_parse_args_int_bounded(user, GUAC_TELNET_CLIENT_ARGS, argv, + IDX_WOL_UDP_PORT, GUAC_WOL_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Parse the WoL wait time. */ settings->wol_wait_time = diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 6c965f7a66..981ef19f9a 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -25,9 +25,11 @@ #include "settings.h" #include +#include #include #include +#include #include #include #include @@ -452,9 +454,9 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, IDX_HOSTNAME, ""); - settings->port = - guac_user_parse_args_int(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_PORT, 0); + settings->port = (unsigned short) + guac_user_parse_args_int_bounded(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_PORT, 0, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); settings->username = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, @@ -523,9 +525,9 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, IDX_DEST_HOST, NULL); /* VNC repeater port */ - settings->dest_port = - guac_user_parse_args_int(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_DEST_PORT, 0); + settings->dest_port = (unsigned short) + guac_user_parse_args_int_bounded(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_DEST_PORT, 0, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); #endif /* Set encodings if specified */ @@ -587,8 +589,8 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, /* Port for SFTP connection */ settings->sftp_port = - guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_SFTP_PORT, "22"); + guac_user_parse_args_int_string_bounded(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_SFTP_PORT, "22", GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* SFTP connection timeout */ settings->sftp_timeout = @@ -715,8 +717,8 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, /* Parse the WoL broadcast port. */ settings->wol_udp_port = (unsigned short) - guac_user_parse_args_int(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_WOL_UDP_PORT, GUAC_WOL_PORT); + guac_user_parse_args_int_bounded(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_WOL_UDP_PORT, GUAC_WOL_PORT, GUAC_ITOA_USHORT_MIN, GUAC_ITOA_USHORT_MAX); /* Parse the WoL wait time. */ settings->wol_wait_time = diff --git a/src/protocols/vnc/settings.h b/src/protocols/vnc/settings.h index f433df9bf9..9779547ca5 100644 --- a/src/protocols/vnc/settings.h +++ b/src/protocols/vnc/settings.h @@ -47,7 +47,7 @@ typedef struct guac_vnc_settings { /** * The port of the VNC server (or repeater) to connect to. */ - int port; + unsigned short port; /** * The username given in the arguments. @@ -112,7 +112,7 @@ typedef struct guac_vnc_settings { /** * The VNC port to connect to, if using a repeater. */ - int dest_port; + unsigned short dest_port; #endif #ifdef ENABLE_VNC_LISTEN diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index 8f5fa9129f..539979c5d6 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -361,13 +361,12 @@ void* guac_vnc_client_thread(void* data) { */ if (settings->wol_wait_time > 0) { guac_client_log(client, GUAC_LOG_DEBUG, "Sending Wake-on-LAN packet, " - "and pausing for %d seconds.", settings->wol_wait_time); + "and retrying connection check %d times every %d seconds.", + GUAC_WOL_DEFAULT_CONNECT_RETRIES, settings->wol_wait_time); - /* char representation of a port should be, at most, 5 characters plus terminator. */ - char* str_port = guac_mem_alloc(6); - if (guac_itoa(str_port, settings->port) < 1) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to convert port to integer for WOL function."); - guac_mem_free(str_port); + char str_port[GUAC_USHORT_STRING_BUFSIZE]; + if (guac_itoa_safe(str_port, sizeof(str_port), settings->port) < 1) { + guac_client_log(client, GUAC_LOG_ERROR, "Failed to convert port to string for WOL function."); return NULL; } @@ -378,26 +377,27 @@ void* guac_vnc_client_thread(void* data) { settings->wol_wait_time, GUAC_WOL_DEFAULT_CONNECT_RETRIES, settings->hostname, - (const char *) str_port, + str_port, GUAC_WOL_DEFAULT_CONNECTION_TIMEOUT)) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet or connect to remote system."); - guac_mem_free(str_port); + guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet, or connect to remote server."); return NULL; } - - guac_mem_free(str_port); - } - /* Just send the packet and continue the connection, or return if failed. */ - else if(guac_wol_wake(settings->wol_mac_addr, + /* Just send the packet, or return NULL if failed. */ + else { + guac_client_log(client, GUAC_LOG_DEBUG, "Sending RDP Wake-on-LAN packet."); + + if(guac_wol_wake(settings->wol_mac_addr, settings->wol_broadcast_addr, settings->wol_udp_port)) { - guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet."); - return NULL; + guac_client_log(client, GUAC_LOG_ERROR, "Failed to send WOL packet."); + return NULL; + } } + } - + /* Configure clipboard encoding */ if (guac_vnc_set_clipboard_encoding(client, settings->clipboard_encoding)) { guac_client_log(client, GUAC_LOG_INFO, "Using non-standard VNC "