From 610ab2dafac4668732733009b079ebcf585dee9f Mon Sep 17 00:00:00 2001 From: "coypu@sdf.org" Date: Sat, 11 Aug 2018 22:28:14 +0000 Subject: [PATCH 1/4] Use-after-frees Hi folks! I tried clang-static-analyzer on irssi 1.1.1, it seems like it finds some things. Here's a diff, but there might be more that you would want to check, or choose to work differently. (in special-vars.c, ret is commands->data sometime) I hope it's not too much trouble if reported as a confidential bug. Thanks. --- src/core/modules.c | 3 ++- src/core/rawlog.c | 3 ++- src/core/special-vars.c | 2 +- src/fe-common/core/themes.c | 3 ++- src/irc/dcc/dcc.c | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/core/modules.c b/src/core/modules.c index a2542c84..4b7a91ca 100644 --- a/src/core/modules.c +++ b/src/core/modules.c @@ -289,8 +289,9 @@ void modules_deinit(void) while (list != NULL) { module_uniq_destroy(list->data); - g_free(list->data); + gconstpointer tmp = list->data; list = g_slist_remove(list, list->data); + g_free(tmp); } g_hash_table_destroy(idlookup); diff --git a/src/core/rawlog.c b/src/core/rawlog.c index fdd51241..da63fa50 100644 --- a/src/core/rawlog.c +++ b/src/core/rawlog.c @@ -64,9 +64,10 @@ static void rawlog_add(RAWLOG_REC *rawlog, char *str) if (rawlog->nlines < rawlog_lines || rawlog_lines <= 2) rawlog->nlines++; else { - g_free(rawlog->lines->data); + gconstpointer tmp = rawlog->lines->data; rawlog->lines = g_slist_remove(rawlog->lines, rawlog->lines->data); + g_free(tmp); } if (rawlog->logging) { diff --git a/src/core/special-vars.c b/src/core/special-vars.c index 33d9cd55..7081b602 100644 --- a/src/core/special-vars.c +++ b/src/core/special-vars.c @@ -620,8 +620,8 @@ void eval_special_string(const char *cmd, const char *data, /* FIXME: window item would need reference counting as well, eg. "/EVAL win close;say hello" wouldn't work now.. */ - g_free(ret); commands = g_slist_remove(commands, commands->data); + g_free(ret); } g_free(orig); } diff --git a/src/fe-common/core/themes.c b/src/fe-common/core/themes.c index e9818be9..cb3a7717 100644 --- a/src/fe-common/core/themes.c +++ b/src/fe-common/core/themes.c @@ -1424,8 +1424,9 @@ void themes_reload(void) change_theme(settings_get_str("theme"), FALSE); while (refs != NULL) { - theme_unref(refs->data); + gconstpointer tmp = refs->data; refs = g_slist_remove(refs, refs->data); + theme_unref(tmp); } } diff --git a/src/irc/dcc/dcc.c b/src/irc/dcc/dcc.c index c51f7c7a..eca4fe64 100644 --- a/src/irc/dcc/dcc.c +++ b/src/irc/dcc/dcc.c @@ -57,8 +57,8 @@ void dcc_unregister_type(const char *type) pos = gslist_find_string(dcc_types, type); if (pos != NULL) { - g_free(pos->data); dcc_types = g_slist_remove(dcc_types, pos->data); + g_free(pos->data); } } From 9dd836b87663929ae540b6b3c724c53b24e512ba Mon Sep 17 00:00:00 2001 From: dequis Date: Fri, 24 Aug 2018 02:11:22 -0300 Subject: [PATCH 2/4] =?UTF-8?q?Fix=20"discards=20=E2=80=98const=E2=80=99?= =?UTF-8?q?=20qualifier"=20warnings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/modules.c | 2 +- src/core/rawlog.c | 2 +- src/fe-common/core/themes.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/modules.c b/src/core/modules.c index 4b7a91ca..8439b73d 100644 --- a/src/core/modules.c +++ b/src/core/modules.c @@ -289,7 +289,7 @@ void modules_deinit(void) while (list != NULL) { module_uniq_destroy(list->data); - gconstpointer tmp = list->data; + void *tmp = list->data; list = g_slist_remove(list, list->data); g_free(tmp); } diff --git a/src/core/rawlog.c b/src/core/rawlog.c index da63fa50..e6bb7017 100644 --- a/src/core/rawlog.c +++ b/src/core/rawlog.c @@ -64,7 +64,7 @@ static void rawlog_add(RAWLOG_REC *rawlog, char *str) if (rawlog->nlines < rawlog_lines || rawlog_lines <= 2) rawlog->nlines++; else { - gconstpointer tmp = rawlog->lines->data; + void *tmp = rawlog->lines->data; rawlog->lines = g_slist_remove(rawlog->lines, rawlog->lines->data); g_free(tmp); diff --git a/src/fe-common/core/themes.c b/src/fe-common/core/themes.c index cb3a7717..a95f6180 100644 --- a/src/fe-common/core/themes.c +++ b/src/fe-common/core/themes.c @@ -1424,7 +1424,7 @@ void themes_reload(void) change_theme(settings_get_str("theme"), FALSE); while (refs != NULL) { - gconstpointer tmp = refs->data; + void *tmp = refs->data; refs = g_slist_remove(refs, refs->data); theme_unref(tmp); } From ade2f87fe599773886a044b7c29a482622dfaea4 Mon Sep 17 00:00:00 2001 From: dequis Date: Fri, 24 Aug 2018 02:19:42 -0300 Subject: [PATCH 3/4] Fix use after free introduced by the use after free patch --- src/irc/dcc/dcc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/irc/dcc/dcc.c b/src/irc/dcc/dcc.c index eca4fe64..01ab0e59 100644 --- a/src/irc/dcc/dcc.c +++ b/src/irc/dcc/dcc.c @@ -57,8 +57,9 @@ void dcc_unregister_type(const char *type) pos = gslist_find_string(dcc_types, type); if (pos != NULL) { - dcc_types = g_slist_remove(dcc_types, pos->data); - g_free(pos->data); + void *tmp = pos->data; + dcc_types = g_slist_remove(dcc_types, pos->data); + g_free(tmp); } } From e450a7f8c1c74132625d0a884f0ca851abd69197 Mon Sep 17 00:00:00 2001 From: dequis Date: Fri, 24 Aug 2018 02:27:45 -0300 Subject: [PATCH 4/4] modules_deinit: Fix -Werror=declaration-after-statement --- src/core/modules.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/modules.c b/src/core/modules.c index 8439b73d..9b531899 100644 --- a/src/core/modules.c +++ b/src/core/modules.c @@ -288,8 +288,8 @@ void modules_deinit(void) g_hash_table_foreach(stridlookup, (GHFunc) uniq_get_modules, &list); while (list != NULL) { - module_uniq_destroy(list->data); void *tmp = list->data; + module_uniq_destroy(list->data); list = g_slist_remove(list, list->data); g_free(tmp); }