Buffer overflow issue in UnrealIRCd, despite use of strncat

Today I had to release UnrealIRCd 4.0.11 due to a buffer overflow issue. In this article I give more information on the bug and exploitability.

To UnrealIRCd users: You can upgrade the IRCd without a restart

The bug

First, the actual issue. This code is present in a for loop:

(void)strncat(buf, s, sizeof(buf) - (len+1));
len += strlen(s);
(void)strncat(buf, " ", sizeof(buf) - (len+1));
len++;

I didn't write this code but the idea here is to add a string s to a buffer buf with proper size checking.

First thing you should know is that buf is 512 bytes:

#define        BUFSIZE         512     /* WARNING: *DONT* CHANGE THIS!!!! */

static char buf[BUFSIZE];

Let's walk through the code step by step. Say we have had a few iterations of the for loop and buf is now 510 bytes full at this point. s contains the string "blah" which needs to be added to buf. Now the first strncat gets executed:

(void)strncat(buf, s, sizeof(buf) - (len+1));

This line will only add 1 character (the terminating nul). All thanks to strncat size checking: sizeof(buf) - (len+1) is 512-(510+1) = 1. No problem here, good!

Unfortunately now comes the next line:

len += strlen(s)

This will increase len by the would-be-appended-string ("blah", so 4 bytes), rather than the actually appended size (0 or 1 bytes). This means len is now bumped +4 from 510 to 514.

Now the next strncat is executed:

(void)strncat(buf, " ", sizeof(buf) - (len+1));

The sizeof(buf) - (len+1) is 512-(514+1)=-3. So a negative number. Unfortunately the 3rd argument to strncat is of size_t which is actually unsigned. This means it is not interpreted as -3 but actually by 2^32-3, a value of more than 4 billion.

It basically means the strncat now executes like this:

(void)strncat(buf, " ", 4294967293);

As you can see, all strncat calls are worthless now. It now behaves like a simple strcat. Next iteration of the for loop we can append any string without size checking.

The fix

One could tweak the strncat line, but a better fix is to use strlcat instead, which is what I did. With strlcat the 3rd size argument is no longer 'maximum to add'-bytes but rather 'maximum buffer'-bytes. So the code is now simply...

strlcat(buf, s, sizeof(buf));
strlcat(buf, " ", sizeof(buf));

...which is must cleaner.

How exploitable is this?

Because input is limited to 510 bytes as well, the ability to overflow is limited. I'll leave out the details but in practice one can overflow the buffer about ~60 bytes under normal circumstances. Maybe up to ~120 in unusual circumstances.

First of all it's important to know where buf is located. It's slightly unusual in this case that buf is neither on the stack nor on the heap. It's a global variable. And in our case it's a global variable within the m_ison module.

A module what? Well, UnrealIRCd uses modules heavily, it loads a lot of commands in their own .so files (dynamically loaded object files). This allows users to enable/disable features on the fly and also fix things (including this bug) without a restart, without any downtime. The .so is simply unloaded/loaded/reloaded.

Let's find out the exact address of buf:

(gdb) p &buf
$2 = (char (*)[512]) 0x7ffff386e600 <buf>

This is the relevant memory map entry (from /proc/pid/maps):

7ffff386e000-7ffff386f000 rw-p 00001000 fd:00 5685422  /home/syzop/unrealircd/tmp/1034945E.m_ison.so

As you can see the address of buf 0x7ffff386e600 is located about halfway on an isolated 4k memory page.

Knowing that buf is 512 bytes, what is directly behind it? Where will we overflow into?

(gdb) x/a buf
0x7ffff386e600 <buf>:   0x0
(gdb) x/a buf+512
0x7ffff386e800 <global_count>:  0x0
(gdb) x/a buf+516
0x7ffff386e804 <max_global_count>:      0x0
(gdb) x/a buf+520
0x7ffff386e808 <now>:   0x0
(gdb) x/a buf+524
0x7ffff386e80c <now+4>: 0x5609000000000000
(gdb) x/a buf+528
0x7ffff386e810: 0x1e556090000
(gdb) x/a buf+532
0x7ffff386e814: 0x2b7a02000001e5
(gdb)

As you can see only global_count, max_global_count and now are directly behind it. These variables are caused by a missing 'extern' in one of the header files. They are never used. Overflowing them is harmless.

It seems there is only garbage after these variables. I can write a full 1024 bytes in buf and everything after it without any damage.

So.. we're lucky this time. Or unlucky, depending on your perspective ;)

Overflow protection

Versions before UnrealIRCd 4.0.8 also have this overflow issue but it simply goes undetected due to the buffer overflowing in unused space.

UnrealIRCd 4.0.8 and later have various "hardening" options enabled such as ones guarding against buffer overflows. This is why version 4.0.8 - 4.0.10 actually detect the buffer overflow issue and halt execution. I'll spend another article on it when I have time.