From 00c80cb6fcca40cfc421fe3fc181115ac4907191 Mon Sep 17 00:00:00 2001 From: ailin-nemui Date: Sat, 7 Oct 2017 20:45:13 +0200 Subject: fix out of bounds read in compress_colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by Hanno Böck. Fixes GL#12 --- src/fe-common/core/themes.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/fe-common/core/themes.c b/src/fe-common/core/themes.c index 2b1459be..cb1cce8f 100644 --- a/src/fe-common/core/themes.c +++ b/src/fe-common/core/themes.c @@ -587,7 +587,7 @@ static char *theme_format_compress_colors(THEME_REC *theme, const char *format) /* a normal character */ g_string_append_c(str, *format); format++; - } else { + } else if (format[1] != '\0') { /* %format */ format++; if (IS_OLD_FORMAT(*format, last_fg, last_bg)) { @@ -614,6 +614,11 @@ static char *theme_format_compress_colors(THEME_REC *theme, const char *format) last_bg = '\0'; } format++; + } else { + /* % at end of string */ + format++; + g_string_append_c(str, '%'); + g_string_append_c(str, '%'); } } -- cgit v1.2.3 From 49ace3251b79a9e97c6e4d0bc640f9143dc71b90 Mon Sep 17 00:00:00 2001 From: ailin-nemui Date: Sun, 8 Oct 2017 19:47:50 +0200 Subject: fix uaf in chanquery module the chanquery needs to be removed in any case if a channel rec is destroyed, regardless of any state Fixes GL#13 --- src/irc/core/channels-query.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/irc/core/channels-query.c b/src/irc/core/channels-query.c index 857ebaf0..d161aec1 100644 --- a/src/irc/core/channels-query.c +++ b/src/irc/core/channels-query.c @@ -125,15 +125,15 @@ static void query_remove_all(IRC_CHANNEL_REC *channel) rec->queries[n] = g_slist_remove(rec->queries[n], channel); rec->current_queries = g_slist_remove(rec->current_queries, channel); - query_check(channel->server); + if (!channel->server->disconnected) + query_check(channel->server); } static void sig_channel_destroyed(IRC_CHANNEL_REC *channel) { g_return_if_fail(channel != NULL); - if (IS_IRC_CHANNEL(channel) && !channel->server->disconnected && - !channel->synced) + if (IS_IRC_CHANNEL(channel)) query_remove_all(channel); } -- cgit v1.2.3 From 2edd816e7db13b4ac0b20df9bf7fe55ee7718215 Mon Sep 17 00:00:00 2001 From: Joseph Bisch Date: Sun, 8 Oct 2017 22:02:44 -0400 Subject: Fix segfault in query_remove_all It is possible for rec to be NULL in query_remove_all, resulting in a segfault. So return without doing anything if rec is NULL. --- src/irc/core/channels-query.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/irc/core/channels-query.c b/src/irc/core/channels-query.c index d161aec1..d7dadf04 100644 --- a/src/irc/core/channels-query.c +++ b/src/irc/core/channels-query.c @@ -119,6 +119,7 @@ static void query_remove_all(IRC_CHANNEL_REC *channel) int n; rec = channel->server->chanqueries; + if (rec == NULL) return; /* remove channel from query lists */ for (n = 0; n < CHANNEL_QUERIES; n++) -- cgit v1.2.3 From 45dfe2ba3889c5dc23a9bea3214f158cc651a043 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 19 Oct 2017 11:17:56 +0200 Subject: Prevent a OOB read when parsing IRCNet ! channels Make sure the string has enough data. Fixes #16 --- src/irc/core/channel-events.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/irc/core/channel-events.c b/src/irc/core/channel-events.c index 6cb9b088..b0bddab2 100644 --- a/src/irc/core/channel-events.c +++ b/src/irc/core/channel-events.c @@ -37,7 +37,7 @@ static void check_join_failure(IRC_SERVER_REC *server, const char *channel) channel++; /* server didn't understand !channels */ chanrec = channel_find(SERVER(server), channel); - if (chanrec == NULL && channel[0] == '!') { + if (chanrec == NULL && channel[0] == '!' && strlen(channel) > 6) { /* it probably replied with the full !channel name, find the channel with the short name.. */ chan2 = g_strdup_printf("!%s", channel+6); -- cgit v1.2.3 From 9f0dc4766c7aa80e34aa2cde94323fb49971abdf Mon Sep 17 00:00:00 2001 From: ailin-nemui Date: Fri, 13 Oct 2017 17:54:57 +0200 Subject: fix dcc issue --- src/irc/dcc/dcc-chat.c | 21 +++++++++++++++++++++ src/irc/dcc/dcc-get.c | 17 +++++++++++++++-- src/irc/dcc/dcc-send.c | 10 ++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/irc/dcc/dcc-chat.c b/src/irc/dcc/dcc-chat.c index ca90b8d8..88c577f7 100644 --- a/src/irc/dcc/dcc-chat.c +++ b/src/irc/dcc/dcc-chat.c @@ -66,6 +66,13 @@ CHAT_DCC_REC *dcc_chat_create(IRC_SERVER_REC *server, dcc->id = dcc_chat_get_new_id(nick); dcc_init_rec(DCC(dcc), server, chat, nick, arg); + if (dcc->module_data == NULL) { + /* failed to successfully init; TODO: change init_rec API */ + g_free(dcc->id); + g_free(dcc); + return NULL; + } + return dcc; } @@ -471,6 +478,7 @@ static void cmd_dcc_chat(const char *data, IRC_SERVER_REC *server) /* We are accepting a passive DCC CHAT. */ dcc_chat_passive(dcc); } + cmd_params_free(free_arg); return; } @@ -485,6 +493,11 @@ static void cmd_dcc_chat(const char *data, IRC_SERVER_REC *server) cmd_param_error(CMDERR_NOT_CONNECTED); dcc = dcc_chat_create(server, NULL, nick, "chat"); + if (dcc == NULL) { + cmd_params_free(free_arg); + g_warn_if_reached(); + return; + } if (g_hash_table_lookup(optlist, "passive") == NULL) { /* Standard DCC CHAT... let's listen for incoming connections */ @@ -627,6 +640,9 @@ static void ctcp_msg_dcc_chat(IRC_SERVER_REC *server, const char *data, } passive = paramcount == 4 && g_strcmp0(params[2], "0") == 0; + if (nick == NULL) + nick = ""; + dcc = DCC_CHAT(dcc_find_request(DCC_CHAT_TYPE, nick, NULL)); if (dcc != NULL) { if (dcc_is_listening(dcc)) { @@ -658,6 +674,11 @@ static void ctcp_msg_dcc_chat(IRC_SERVER_REC *server, const char *data, } dcc = dcc_chat_create(server, chat, nick, params[0]); + if (dcc == NULL) { + g_strfreev(params); + g_warn_if_reached(); + return; + } dcc->target = g_strdup(target); dcc->port = atoi(params[2]); diff --git a/src/irc/dcc/dcc-get.c b/src/irc/dcc/dcc-get.c index 107f68fa..cecbb076 100644 --- a/src/irc/dcc/dcc-get.c +++ b/src/irc/dcc/dcc-get.c @@ -43,6 +43,12 @@ GET_DCC_REC *dcc_get_create(IRC_SERVER_REC *server, CHAT_DCC_REC *chat, dcc->fhandle = -1; dcc_init_rec(DCC(dcc), server, chat, nick, arg); + if (dcc->module_data == NULL) { + /* failed to successfully init; TODO: change API */ + g_free(dcc); + return NULL; + } + return dcc; } @@ -430,9 +436,10 @@ static void ctcp_msg_dcc_send(IRC_SERVER_REC *server, const char *data, int p_id = -1; int passive = FALSE; - if (addr == NULL) { + if (addr == NULL) addr = ""; - } + if (nick == NULL) + nick = ""; /* SEND
[...] */ /* SEND
0 (DCC SEND passive protocol) */ @@ -512,6 +519,12 @@ static void ctcp_msg_dcc_send(IRC_SERVER_REC *server, const char *data, dcc_destroy(DCC(dcc)); /* remove the old DCC */ dcc = dcc_get_create(server, chat, nick, fname); + if (dcc == NULL) { + g_free(address); + g_free(fname); + g_warn_if_reached(); + return; + } dcc->target = g_strdup(target); if (passive && port == 0) diff --git a/src/irc/dcc/dcc-send.c b/src/irc/dcc/dcc-send.c index ca29b9b9..912129b7 100644 --- a/src/irc/dcc/dcc-send.c +++ b/src/irc/dcc/dcc-send.c @@ -237,6 +237,12 @@ static SEND_DCC_REC *dcc_send_create(IRC_SERVER_REC *server, dcc->queue = -1; dcc_init_rec(DCC(dcc), server, chat, nick, arg); + if (dcc->module_data == NULL) { + /* failed to successfully init; TODO: change API */ + g_free(dcc); + return NULL; + } + return dcc; } @@ -417,6 +423,10 @@ static int dcc_send_one_file(int queue, const char *target, const char *fname, dcc = dcc_send_create(server, chat, target, str); g_free(str); + if (dcc == NULL) { + g_warn_if_reached(); + return FALSE; + } dcc->handle = handle; dcc->port = port; -- cgit v1.2.3 From 73d7b9d7753d35c63f24defe6d26c7c06ffa3cce Mon Sep 17 00:00:00 2001 From: Joseph Bisch Date: Mon, 16 Oct 2017 16:21:10 -0400 Subject: Don't proceed with cmd_msg if there was an error splitting msg There may be cases (such as if target or server->nick is very long) where the split_message function returns NULL, indicating an error. To avoid a potential segfault, we now check to see if splitmsgs is NULL. --- src/core/chat-commands.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/chat-commands.c b/src/core/chat-commands.c index d5a133f8..77f02aa2 100644 --- a/src/core/chat-commands.c +++ b/src/core/chat-commands.c @@ -404,7 +404,10 @@ static void cmd_msg(const char *data, SERVER_REC *server, WI_ITEM_REC *item) else splitmsgs = singlemsg; - while ((m = splitmsgs[n++])) { + /* splitmsgs may be NULL if there was an error */ + g_warn_if_fail(splitmsgs != NULL); + + while (splitmsgs && (m = splitmsgs[n++])) { signal_emit("server sendmsg", 4, server, target, m, GINT_TO_POINTER(target_type)); signal_emit(target_type == SEND_TARGET_CHANNEL ? -- cgit v1.2.3 From beb2beba3b4802c6969a5595197e25e7a5483fa3 Mon Sep 17 00:00:00 2001 From: Joseph Bisch Date: Wed, 18 Oct 2017 14:33:02 -0400 Subject: Revert "Don't proceed with cmd_msg if there was an error splitting msg" This reverts commit bd83852d646de28f2e0fe01efe7c9236aa4074d4. --- src/core/chat-commands.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'src') diff --git a/src/core/chat-commands.c b/src/core/chat-commands.c index 77f02aa2..d5a133f8 100644 --- a/src/core/chat-commands.c +++ b/src/core/chat-commands.c @@ -404,10 +404,7 @@ static void cmd_msg(const char *data, SERVER_REC *server, WI_ITEM_REC *item) else splitmsgs = singlemsg; - /* splitmsgs may be NULL if there was an error */ - g_warn_if_fail(splitmsgs != NULL); - - while (splitmsgs && (m = splitmsgs[n++])) { + while ((m = splitmsgs[n++])) { signal_emit("server sendmsg", 4, server, target, m, GINT_TO_POINTER(target_type)); signal_emit(target_type == SEND_TARGET_CHANNEL ? -- cgit v1.2.3 From 0840eaec7bf56740029aae614e393f8cf76f6946 Mon Sep 17 00:00:00 2001 From: Joseph Bisch Date: Wed, 18 Oct 2017 14:52:04 -0400 Subject: Make split functions return an array with NULL instead of NULL This avoids undefined behavior in functions that call these split functions and expect an array back instead of just a NULL pointer. --- src/core/recode.c | 7 ++++++- src/irc/core/irc-servers.c | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/core/recode.c b/src/core/recode.c index d001a46a..d3fc91e7 100644 --- a/src/core/recode.c +++ b/src/core/recode.c @@ -198,7 +198,12 @@ char **recode_split(const SERVER_REC *server, const char *str, int n = 0; char **ret; - g_return_val_if_fail(str != NULL, NULL); + g_warn_if_fail(str != NULL); + if (str == NULL) { + ret = g_new(char *, 1); + ret[0] = NULL; + return ret; + } if (settings_get_bool("recode")) { to = find_conversion(server, target); diff --git a/src/irc/core/irc-servers.c b/src/irc/core/irc-servers.c index 3117e345..4eaab712 100644 --- a/src/irc/core/irc-servers.c +++ b/src/irc/core/irc-servers.c @@ -116,11 +116,14 @@ static char **split_line(const SERVER_REC *server, const char *line, * the code much simpler. It's worth it. */ len -= strlen(recoded_start) + strlen(recoded_end); + g_warn_if_fail(len > 0); if (len <= 0) { /* There is no room for anything. */ g_free(recoded_start); g_free(recoded_end); - return NULL; + lines = g_new(char *, 1); + lines[0] = NULL; + return lines; } lines = recode_split(server, line, target, len, onspace); -- cgit v1.2.3