From d9ea224628af1a4394fec20610e136a3c6817f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Sun, 28 Sep 2014 15:48:48 +0200 Subject: [PATCH] Fix use-after-free bug with cached settings values This patch fixes a couple of use-after-free bugs when caching various string related setting values. Fixes: #143 --- src/core/expandos.c | 22 ++++++++++++++-------- src/core/log.c | 20 ++++++++++++-------- src/fe-common/core/chat-completion.c | 14 +++++++++++--- src/fe-common/core/fe-log.c | 24 +++++++++++++++++------- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/core/expandos.c b/src/core/expandos.c index bed6c5eb..1fc517af 100644 --- a/src/core/expandos.c +++ b/src/core/expandos.c @@ -57,7 +57,7 @@ static char *last_sent_msg, *last_sent_msg_body; static char *last_privmsg_from, *last_public_from; static char *sysname, *sysrelease, *sysarch; -static const char *timestamp_format; +static char *timestamp_format; static int timestamp_seconds; static time_t last_timestamp; @@ -567,7 +567,9 @@ static int sig_timer(void) static void read_settings(void) { - timestamp_format = settings_get_str("timestamp_format"); + g_free_not_null(timestamp_format); + timestamp_format = g_strdup(settings_get_str("timestamp_format")); + timestamp_seconds = strstr(timestamp_format, "%r") != NULL || strstr(timestamp_format, "%s") != NULL || @@ -708,14 +710,18 @@ void expandos_deinit(void) g_free_not_null(char_expandos[n]); g_hash_table_foreach_remove(expandos, free_expando, NULL); - g_hash_table_destroy(expandos); + g_hash_table_destroy(expandos); - g_free_not_null(last_sent_msg); g_free_not_null(last_sent_msg_body); - g_free_not_null(last_privmsg_from); g_free_not_null(last_public_from); - g_free_not_null(sysname); g_free_not_null(sysrelease); - g_free_not_null(sysarch); + g_free_not_null(last_sent_msg); + g_free_not_null(last_sent_msg_body); + g_free_not_null(last_privmsg_from); + g_free_not_null(last_public_from); + g_free_not_null(sysname); + g_free_not_null(sysrelease); + g_free_not_null(sysarch); + g_free_not_null(timestamp_format); - g_source_remove(timer_tag); + g_source_remove(timer_tag); signal_remove("message public", (SIGNAL_FUNC) sig_message_public); signal_remove("message private", (SIGNAL_FUNC) sig_message_private); signal_remove("message own_private", (SIGNAL_FUNC) sig_message_own_private); diff --git a/src/core/log.c b/src/core/log.c index 263b3526..d4d3853e 100644 --- a/src/core/log.c +++ b/src/core/log.c @@ -41,7 +41,7 @@ static const char *log_item_types[] = { NULL }; -const char *log_timestamp; +static char *log_timestamp; static int log_file_create_mode; static int log_dir_create_mode; static int rotate_tag; @@ -558,13 +558,15 @@ static void log_read_config(void) static void read_settings(void) { - log_timestamp = settings_get_str("log_timestamp"); + g_free_not_null(log_timestamp); + log_timestamp = g_strdup(settings_get_str("log_timestamp")); + log_file_create_mode = octal2dec(settings_get_int("log_create_mode")); - log_dir_create_mode = log_file_create_mode; - if (log_file_create_mode & 0400) log_dir_create_mode |= 0100; - if (log_file_create_mode & 0040) log_dir_create_mode |= 0010; - if (log_file_create_mode & 0004) log_dir_create_mode |= 0001; + log_dir_create_mode = log_file_create_mode; + if (log_file_create_mode & 0400) log_dir_create_mode |= 0100; + if (log_file_create_mode & 0040) log_dir_create_mode |= 0010; + if (log_file_create_mode & 0004) log_dir_create_mode |= 0001; } void log_init(void) @@ -595,7 +597,9 @@ void log_deinit(void) while (logs != NULL) log_close(logs->data); + g_free_not_null(log_timestamp); + signal_remove("setup changed", (SIGNAL_FUNC) read_settings); - signal_remove("setup reread", (SIGNAL_FUNC) log_read_config); - signal_remove("irssi init finished", (SIGNAL_FUNC) log_read_config); + signal_remove("setup reread", (SIGNAL_FUNC) log_read_config); + signal_remove("irssi init finished", (SIGNAL_FUNC) log_read_config); } diff --git a/src/fe-common/core/chat-completion.c b/src/fe-common/core/chat-completion.c index aba76b9d..c37c77cd 100644 --- a/src/fe-common/core/chat-completion.c +++ b/src/fe-common/core/chat-completion.c @@ -40,7 +40,7 @@ static int keep_privates_count, keep_publics_count; static int completion_lowercase; -static const char *completion_char, *cmdchars; +static char *completion_char, *cmdchars; static GSList *global_lastmsgs; static int completion_auto, completion_strict; @@ -1126,11 +1126,16 @@ static void read_settings(void) keep_privates_count = settings_get_int("completion_keep_privates"); keep_publics_count = settings_get_int("completion_keep_publics"); completion_lowercase = settings_get_bool("completion_nicks_lowercase"); - completion_char = settings_get_str("completion_char"); - cmdchars = settings_get_str("cmdchars"); + completion_auto = settings_get_bool("completion_auto"); completion_strict = settings_get_bool("completion_strict"); + g_free_not_null(completion_char); + completion_char = g_strdup(settings_get_str("completion_char")); + + g_free_not_null(cmdchars); + cmdchars = g_strdup(settings_get_str("cmdchars")); + if (*completion_char == '\0') { /* this would break.. */ completion_auto = FALSE; @@ -1220,4 +1225,7 @@ void chat_completion_deinit(void) signal_remove("server disconnected", (SIGNAL_FUNC) sig_server_disconnected); signal_remove("channel destroyed", (SIGNAL_FUNC) sig_channel_destroyed); signal_remove("setup changed", (SIGNAL_FUNC) read_settings); + + g_free_not_null(completion_char); + g_free_not_null(cmdchars); } diff --git a/src/fe-common/core/fe-log.c b/src/fe-common/core/fe-log.c index 5ee72d8b..9d68faf9 100644 --- a/src/fe-common/core/fe-log.c +++ b/src/fe-common/core/fe-log.c @@ -43,11 +43,11 @@ static int autolog_level; static int autoremove_tag; -static const char *autolog_path; +static char *autolog_path; static THEME_REC *log_theme; static int skip_next_printtext; -static const char *log_theme_name; +static char *log_theme_name; static int log_dir_create_mode; @@ -675,9 +675,11 @@ static void sig_theme_destroyed(THEME_REC *theme) static void read_settings(void) { int old_autolog = autolog_level; - int log_file_create_mode; + int log_file_create_mode; + + g_free_not_null(autolog_path); + autolog_path = g_strdup(settings_get_str("autolog_path")); - autolog_path = settings_get_str("autolog_path"); autolog_level = !settings_get_bool("autolog") ? 0 : settings_get_level("autolog_level"); @@ -687,9 +689,14 @@ static void read_settings(void) /* write to log files with different theme? */ if (log_theme_name != NULL) signal_remove("print format", (SIGNAL_FUNC) sig_print_format); - log_theme_name = settings_get_str("log_theme"); - if (*log_theme_name == '\0') + + g_free_not_null(log_theme_name); + log_theme_name = g_strdup(settings_get_str("log_theme")); + + if (*log_theme_name == '\0') { + g_free(log_theme_name); log_theme_name = NULL; + } else signal_add("print format", (SIGNAL_FUNC) sig_print_format); @@ -752,7 +759,7 @@ void fe_log_deinit(void) { g_source_remove(autoremove_tag); if (log_theme_name != NULL) - signal_remove("print format", (SIGNAL_FUNC) sig_print_format); + signal_remove("print format", (SIGNAL_FUNC) sig_print_format); command_unbind("log", (SIGNAL_FUNC) cmd_log); command_unbind("log open", (SIGNAL_FUNC) cmd_log_open); @@ -776,4 +783,7 @@ void fe_log_deinit(void) if (autolog_ignore_targets != NULL) g_strfreev(autolog_ignore_targets); + + g_free_not_null(autolog_path); + g_free_not_null(log_theme_name); }