From fdd3292f09f37eb12fc71f9d59470a863f9aa590 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 12 Nov 2018 16:10:24 -0800 Subject: [PATCH] GUACAMOLE-637: Simplify path translation logic. Update to use guac_strl*(). Fix return values. --- src/common-ssh/sftp.c | 138 +++++++++++++------------------------ src/protocols/rdp/rdp_fs.c | 28 +++----- src/protocols/rdp/rdp_fs.h | 5 ++ 3 files changed, 64 insertions(+), 107 deletions(-) diff --git a/src/common-ssh/sftp.c b/src/common-ssh/sftp.c index 51fa7cd5..0a24c049 100644 --- a/src/common-ssh/sftp.c +++ b/src/common-ssh/sftp.c @@ -64,12 +64,11 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, char path_component_data[GUAC_COMMON_SSH_SFTP_MAX_PATH]; const char* path_components[GUAC_COMMON_SSH_SFTP_MAX_DEPTH]; - const char** current_path_component = &(path_components[0]); - const char* current_path_component_data = &(path_component_data[0]); + const char* current_path_component_data = &(path_component_data[0]); /* If original path is not absolute, normalization fails */ if (path[0] != '\\' && path[0] != '/') - return 1; + return 0; /* Skip past leading slash */ path++; @@ -80,7 +79,7 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, /* Fail if input path was truncated */ if (length >= sizeof(path_component_data)) - return 1; + return 0; /* Find path components within path */ for (i = 0; i < sizeof(path_component_data); i++) { @@ -100,9 +99,16 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, /* Otherwise, if component not current directory, add to list */ else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) + && strcmp(current_path_component_data, "") != 0) { + + /* Fail normalization if path is too deep */ + if (path_depth >= GUAC_COMMON_SSH_SFTP_MAX_DEPTH) + return 0; + path_components[path_depth++] = current_path_component_data; + } + /* If end of string, stop */ if (c == '\0') break; @@ -121,21 +127,9 @@ static int guac_common_ssh_sftp_normalize_path(char* fullpath, } /* Convert components back into path */ - for (; path_depth > 0; path_depth--) { + guac_strljoin(fullpath, path_components, path_depth, + "/", GUAC_COMMON_SSH_SFTP_MAX_PATH); - const char* filename = *(current_path_component++); - - /* Add separator */ - *(fullpath++) = '/'; - - /* Copy string */ - while (*filename != 0) - *(fullpath++) = *(filename++); - - } - - /* Terminate absolute path */ - *(fullpath++) = 0; return 1; } @@ -232,7 +226,7 @@ static guac_protocol_status guac_sftp_get_status( static int guac_ssh_append_filename(char* fullpath, const char* path, const char* filename) { - int i; + int length; /* Disallow "." as a filename */ if (strcmp(filename, ".") == 0) @@ -242,49 +236,29 @@ static int guac_ssh_append_filename(char* fullpath, const char* path, if (strcmp(filename, "..") == 0) return 0; - /* Copy path, append trailing slash */ - for (i=0; i 0 && path[i-1] != '/') - fullpath[i++] = '/'; - break; - } - - /* Copy character if not end of string */ - fullpath[i] = c; - - } - - /* Append filename */ - for (; i 0 && fullpath[length - 1] != '/') + length += guac_strlcpy(fullpath + length, "/", + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); + + /* Append filename */ + length += guac_strlcpy(fullpath + length, filename, + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); + + /* Verify path length is within maximum */ + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) + return 0; /* Append was successful */ return 1; @@ -313,46 +287,30 @@ static int guac_ssh_append_filename(char* fullpath, const char* path, static int guac_ssh_append_path(char* fullpath, const char* path_a, const char* path_b) { - int i; + int length; - /* Copy path, appending a trailing slash */ - for (i = 0; i < GUAC_COMMON_SSH_SFTP_MAX_PATH; i++) { + /* Copy first half of path */ + length = guac_strlcpy(fullpath, path_a, GUAC_COMMON_SSH_SFTP_MAX_PATH); + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) + return 0; - char c = path_a[i]; - if (c == '\0') { - if (i > 0 && path_a[i-1] != '/') - fullpath[i++] = '/'; - break; - } - - /* Copy character if not end of string */ - fullpath[i] = c; - - } + /* Ensure path ends with trailing slash */ + if (length == 0 || fullpath[length - 1] != '/') + length += guac_strlcpy(fullpath + length, "/", + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); /* Skip past leading slashes in second path */ while (*path_b == '/') path_b++; - /* Append path */ - for (; i < GUAC_COMMON_SSH_SFTP_MAX_PATH; i++) { - - char c = *(path_b++); - if (c == '\0') - break; - - /* Append each character within path */ - fullpath[i] = c; - - } + /* Append final half of path */ + length += guac_strlcpy(fullpath + length, path_b, + GUAC_COMMON_SSH_SFTP_MAX_PATH - length); /* Verify path length is within maximum */ - if (i == GUAC_COMMON_SSH_SFTP_MAX_PATH) + if (length >= GUAC_COMMON_SSH_SFTP_MAX_PATH) return 0; - /* Terminate path string */ - fullpath[i] = '\0'; - /* Append was successful */ return 1; diff --git a/src/protocols/rdp/rdp_fs.c b/src/protocols/rdp/rdp_fs.c index db58c77e..3319e1db 100644 --- a/src/protocols/rdp/rdp_fs.c +++ b/src/protocols/rdp/rdp_fs.c @@ -610,9 +610,8 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { int i; int path_depth = 1; char path_component_data[GUAC_RDP_FS_MAX_PATH]; - const char* path_components[64] = { "" }; + const char* path_components[GUAC_RDP_MAX_PATH_DEPTH] = { "" }; - const char** current_path_component = &(path_components[1]); const char* current_path_component_data = &(path_component_data[0]); /* If original path is not absolute, normalization fails */ @@ -648,9 +647,16 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { /* Otherwise, if component not current directory, add to list */ else if (strcmp(current_path_component_data, ".") != 0 - && strcmp(current_path_component_data, "") != 0) + && strcmp(current_path_component_data, "") != 0) { + + /* Fail normalization if path is too deep */ + if (path_depth >= GUAC_RDP_MAX_PATH_DEPTH) + return 1; + path_components[path_depth++] = current_path_component_data; + } + /* If end of string, stop */ if (c == 0) break; @@ -673,21 +679,9 @@ int guac_rdp_fs_normalize_path(const char* path, char* abs_path) { } /* Convert components back into path */ - for (; path_depth > 0; path_depth--) { + guac_strljoin(abs_path, path_components, path_depth, + "\\", GUAC_RDP_FS_MAX_PATH); - const char* filename = *(current_path_component++); - - /* Add separator */ - *(abs_path++) = '\\'; - - /* Copy string */ - while (*filename != 0) - *(abs_path++) = *(filename++); - - } - - /* Terminate absolute path */ - *(abs_path++) = 0; return 0; } diff --git a/src/protocols/rdp/rdp_fs.h b/src/protocols/rdp/rdp_fs.h index 9168efb1..7ceb5ff9 100644 --- a/src/protocols/rdp/rdp_fs.h +++ b/src/protocols/rdp/rdp_fs.h @@ -50,6 +50,11 @@ */ #define GUAC_RDP_FS_MAX_PATH 4096 +/** + * The maximum number of directories a path may contain. + */ +#define GUAC_RDP_MAX_PATH_DEPTH 64 + /** * Error code returned when no more file IDs can be allocated. */