Irssi Bug #563


While fuzzing irssi’s config file parser using afl, I encountered a bug that I reported and became issue #563 for irssi on GitHub. I am going to perform an analysis of the bug and my solution to the bug.

All I had to do to build irssi with afl was add CC=/path/to/afl/afl-gcc to the configure script invocation. After building irssi, I started running irssi --config file.cfg with afl, allowing afl to pass in the configuration file it generates in place of file.cfg. All that can be done by following a guide that covers how to use afl.

After running afl for a while, I ended up with a number of crashes. The input that caused the first crash minimized to the following:

0 0

Running that input through irssi outside of the context of afl yielded a coredump that I was able to give to gdb to get a backtrace.

I reported the bug, including the minimized input, the backtrace, and the commit I was fuzzing and it became issue #563.

After going through some more of the crashes I found, I ended up with two more minimized inputs that appeared to be caused by the same issue.

Debugging with gdb

We will start with the backtrace:

(gdb) bt
#0  0x00007f2a7a92c693 in g_slist_last () from /usr/lib/
#1  0x00007f2a7a92c6df in g_slist_append () from /usr/lib/
#2  0x0000000000494d62 in config_node_set_str (rec=0x16f6790, parent=parent@entry=0x16f7ac0, key=key@entry=0x4a4100 "type", value=0x16ff490 "IRC") at set.c:119
#3  0x000000000047cf5e in chatnet_read (node=0x16f7ac0) at chatnets.c:146
#4  0x000000000047d14a in read_chatnets () at chatnets.c:174
#5  0x000000000048f083 in signal_emit_real (rec=rec@entry=0x16f84b0, params=params@entry=0, va=va@entry=0x7ffcaf583948, first_hook=<optimized out>) at signals.c:242
#6  0x000000000048f4ae in signal_emit (signal=signal@entry=0x4a617e "irssi init read settings", params=params@entry=0) at signals.c:286
#7  0x000000000043ef5f in fe_common_core_finish_init () at fe-common-core.c:426
#8  0x000000000042a29b in textui_finish_init () at irssi.c:197
#9  0x000000000042a497 in main (argc=<optimized out>, argv=<optimized out>) at irssi.c:314

Seeing that the last function called that is part of irssi itself is config_node_set_str, that looks like a good starting place.

Let’s see what that line looks like:

(gdb) list set.c:119
114			if (g_strcmp0(node->value, value) == 0)
115				return;
116	                g_free(node->value);
117		} else {
118			node = g_new0(CONFIG_NODE, 1);
119			parent->value = g_slist_append(parent->value, node);
121			node->type = no_key ? NODE_TYPE_VALUE : NODE_TYPE_KEY;
122			node->key = no_key ? NULL : g_strdup(key);
123		}

We can see that the g_slist_append is taking in parameters of parent->value and node.

What if we print node?

(gdb) frame 2
#2  0x0000000000494d62 in config_node_set_str (rec=0x16f6790, parent=parent@entry=0x16f7ac0, key=key@entry=0x4a4100 "type", value=0x16ff490 "IRC") at set.c:119
119			parent->value = g_slist_append(parent->value, node);
(gdb) p node
$1 = (CONFIG_NODE *) 0x1795ff0

We see that node is a CONFIG_NODE pointer.

How about parent->value?

(gdb) p parent->value
$2 = (void *) 0x16f7bd0

We get a void pointer. We have no type info about the pointer. Looking at the GLib documentation, it seems that the first parameter to g_slist_append is expected to be a GSList pointer.

What if we interpret the void pointer as a GSList pointer?

(gdb) p (GSList *)(parent->value)
$3 = 0x16f7bd0 = {0x7f2a79f60030, 0x7f2a79f67c58 <main_arena+376>, 0x7f2a79f67c48 <main_arena+360>, 0x7f2a79f67c38 <main_arena+344>, 0x7f2a79f67c28 <main_arena+328>, 0x7f2a79f67c18 <main_arena+312>, 
  0x7f2a79f67c08 <main_arena+296>, 0x7f2a79f67bf8 <main_arena+280>, 0x7f2a79f67be8 <main_arena+264>, 0x7f2a79f67bd8 <main_arena+248>, 0x7f2a79f67bc8 <main_arena+232>, 0x7f2a79f67bb8 <main_arena+216>, 
  0x7f2a79f67ba8 <main_arena+200>, 0x7f2a79f67b98 <main_arena+184>, 0x7f2a79f67b88 <main_arena+168>, 0x7f2a79f67b78 <main_arena+152>, 0x7f2a79f67b68 <main_arena+136>, 0x7f2a79f67b58 <main_arena+120>, 
  0x7f2a79f67b48 <main_arena+104>, 0x1796120, 0x64, Cannot access memory at address 0x141

Hm, interesting. Now lets see what happens if we interpret the void pointer as a char pointer?

(gdb) p (char *)(parent->value)
$4 = 0x16f7bd0 "0"

Ah, so it seems like parent->value points to the character ‘0’ followed by a terminator.

So one solution is to make sure that parent is a list node before assuming that parent->value points to a GSList.

First we need to backtrack and see where parent is coming from:

(gdb) l chatnet_read
130	{
131	        CHAT_PROTOCOL_REC *proto;
132		CHATNET_REC *rec;
133	        char *type;
135		if (node == NULL || node->key == NULL)
136			return;
138		type = config_node_get_str(node, "type", NULL);
139		proto = type == NULL ? NULL : chat_protocol_find(type);
140		if (proto == NULL) {
141			proto = type == NULL ? chat_protocol_get_default() :
142				chat_protocol_get_unknown(type);
143		}
145		if (type == NULL)
146			iconfig_node_set_str(node, "type", proto->name);
148		rec = proto->create_chatnet();
149		rec->type = module_get_uniq_id("CHATNET", 0);

We see the beginning of the chatnet_read function. There is a call to iconfig_node_set_str on line 146, where node is passed as the first parameter, so it will be parent in the context of config_node_set_str. The iconfig_node_* functions just seem to actually be #defines to the corresponding config_node_* functions with the mainconfig passed in as the first parameter.

We see on line 135 a check to ensure that node != NULL and node->key != NULL before proceeding, but there isn’t a check to make sure node is a list node. So I made the following change:

diff --git a/src/core/chatnets.c b/src/core/chatnets.c
index dd4d94b..e0a7a8d 100644
--- a/src/core/chatnets.c
+++ b/src/core/chatnets.c
@@ -132,7 +132,7 @@ static void chatnet_read(CONFIG_NODE *node)
         char *type;
-	if (node == NULL || node->key == NULL)
+	if (node == NULL || node->key == NULL || !is_node_list(node))
 	type = config_node_get_str(node, "type", NULL);

That change is irssi PR #570.

October 30, 2016
841 words


afl irssi github linux

Connect. Socialize.