From e6209afcd6923d9803c3ea6bacf3f9322a7a7450 Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Sun, 15 Jul 2018 14:59:54 +0100 Subject: [PATCH 1/2] Fix config buffer overflow and logic --- sway/config.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/sway/config.c b/sway/config.c index 636f5f574..b8c874e62 100644 --- a/sway/config.c +++ b/sway/config.c @@ -560,8 +560,8 @@ static char *expand_line(const char *block, const char *line, bool add_brace) { bool read_config(FILE *file, struct sway_config *config) { bool reading_main_config = false; - char *this_config = NULL, *config_pos; - long config_size = 0; + char *this_config = NULL; + unsigned long config_size = 0; if (config->current_config == NULL) { reading_main_config = true; @@ -569,7 +569,7 @@ bool read_config(FILE *file, struct sway_config *config) { config_size = ftell(file); rewind(file); - config_pos = this_config = malloc(config_size + 1); + config->current_config = this_config = calloc(1, config_size + 1); if (this_config == NULL) { wlr_log(WLR_ERROR, "Unable to allocate buffer for config contents"); return false; @@ -580,6 +580,7 @@ bool read_config(FILE *file, struct sway_config *config) { int line_number = 0; char *line; list_t *stack = create_list(); + size_t read = 0; while (!feof(file)) { char *block = stack->length ? stack->items[0] : NULL; line = read_line(file); @@ -590,10 +591,21 @@ bool read_config(FILE *file, struct sway_config *config) { wlr_log(WLR_DEBUG, "Read line %d: %s", line_number, line); if (reading_main_config) { - size_t l = strlen(line); - memcpy(config_pos, line, l); // don't copy terminating character - config_pos += l; - *config_pos++ = '\n'; + size_t length = strlen(line); + + if (read + length > config_size) { + wlr_log(WLR_ERROR, "Config file changed during reading"); + list_foreach(stack, free); + list_free(stack); + free(line); + return false; + } + + strcpy(this_config + read, line); + if (line_number != 1) { + this_config[read - 1] = '\n'; + } + read += length + 1; } line = strip_whitespace(line); @@ -616,7 +628,6 @@ bool read_config(FILE *file, struct sway_config *config) { list_foreach(stack, free); list_free(stack); free(line); - free(this_config); return false; } wlr_log(WLR_DEBUG, "Expanded line: %s", expanded); @@ -677,10 +688,6 @@ bool read_config(FILE *file, struct sway_config *config) { list_foreach(stack, free); list_free(stack); - if (reading_main_config) { - this_config[config_size - 1] = '\0'; - config->current_config = this_config; - } return success; } From 011d43746faf1bb2a6619e1246143724eb253b52 Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Sun, 15 Jul 2018 15:36:51 +0100 Subject: [PATCH 2/2] Add error handling for getting config file size --- sway/config.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/sway/config.c b/sway/config.c index b8c874e62..2c0511466 100644 --- a/sway/config.c +++ b/sway/config.c @@ -561,12 +561,17 @@ static char *expand_line(const char *block, const char *line, bool add_brace) { bool read_config(FILE *file, struct sway_config *config) { bool reading_main_config = false; char *this_config = NULL; - unsigned long config_size = 0; + size_t config_size = 0; if (config->current_config == NULL) { reading_main_config = true; - fseek(file, 0, SEEK_END); - config_size = ftell(file); + int ret_seek = fseek(file, 0, SEEK_END); + long ret_tell = ftell(file); + if (ret_seek == -1 || ret_tell == -1) { + wlr_log(WLR_ERROR, "Unable to get size of config file"); + return false; + } + config_size = ret_tell; rewind(file); config->current_config = this_config = calloc(1, config_size + 1);