15th Mar 2003 [SBWID-6067]
COMMAND
ircII-based clients buffer overflows
SYSTEMS AFFECTED
BitchX 1.0c19 ircii 20020912 EPIC4 1.1.7.20020907 EPIC4 1.0.1 XChat
2.0.1
PROBLEM
After seeing the BitchX "DoS" problem mentioned the n'th time already,
I decided to finally audit ircII based clients to show some worse
problems they have. I had been pretty sure for years that malicious
servers can exploit them in multiple ways, and I think many others have
known it as well. EPIC and ircII authors have been working to fix
these, but looks like their job isn't yet finished.
Let's state this clearly first: Regular USERS CANNOT EXPLOIT these
bugs. This means that these clients are safe when they're connected to
standard IRC servers. Connecting to special servers can cause problems
though. So it requires user to type /SERVER evil.server.org to exploit
these bugs. I don't think it's too difficult with a bit of social
engineering though.
Of course, man-in-the-middle can also exploit these.
There may be more problems than what I list below. ircII wasn't
originally written to be safe against malicious servers, so there's a
lot of code that needed fixing. My audit was only a quick look at the
clients, you may well find more.
So, the safest fix is to not connect to untrusted servers, and not IRC
over insecure network links. (Or just switch to some safe non-ircII
based client.)
BitchX 1.0c19
-------------
Full of sprintf() calls and relying on BIG_BUFFER_SIZE being large
enough. There's multiple ways to exploit it by giving
near-BIG_BUFFER_SIZE strings in various places. 1)
---
#define IRCD_BUFFER_SIZE 512
#define BIG_BUFFER_SIZE IRCD_BUFFER_SIZE*4
extern void send_ctcp (int type, char *to, int datatag, char *format, ...)
{
char putbuf [BIG_BUFFER_SIZE + 1],
*putbuf2;
int len;
len = IRCD_BUFFER_SIZE - (12 + strlen(to));
putbuf2 = alloca(len);
...
snprintf(putbuf2, len, "%c%s %s%c",
CTCP_DELIM_CHAR,
ctcp_cmd[datatag].name, putbuf,
CTCP_DELIM_CHAR);
...
putbuf2[len - 2] = CTCP_DELIM_CHAR;
putbuf2[len - 1] = 0;
---
The len part looks interesting. Especially if strlen(to) is larger than
IRCD_BUFFER_SIZE-12, which gives alloca() a negative value (actually
casted into large size_t). alloca() is pretty stupid function and
doesn't perform any bounds checking, so what actually happens is that
it returns a pointer to somewhere inside the stack already in use. You
should be able to overwrite function's return address with a little
testing. I didn't bother enough to verify that, but I got close enough:
Send: ":A*502 PRIVMSG a :^APING X*151^A"
Program received signal SIGSEGV, Segmentation fault.
0x08079e90 in send_ctcp (type=1, to=0xbfffee91 'A' <repeats 200 times>...,
datatag=312, format=0x58585858 <Address 0x58585858 out of bounds>)
at ctcp.c:1513
1513 putbuf2[len - 2] = CTCP_DELIM_CHAR;
(gdb) p /x len
$79 = 0x58585858
Which gives the possibility to write ASCII 1+0 anywhere in memory. That
may be enough for exploit.
2) cannot_join_channel() allows writing one of 6 predefined texts past
buffer in stack if channel name is large enough.
3) cluster() is called by several autokick/userlist/etc. functions.
static char result[] can be overflowed by around 1500 bytes by giving a
long hostname.
4) BX_compress_modes() allows overflowing char nmodes[16] if /set
compress_modes on (default is off)
5) handle_oper_vision(): Looks like feeding invalid "Client connecting"
line could overflow sprintf() in it. Didn't verify.
6) ban_it() uses static char banstr[BIG_BUFFER_SIZE/4+1]. Very likely
overflowable.
Above ones were found just by grepping strcat and sprintf, there might
be more. In general, just think what happens if nick name, host name or
channel name is near BIG_BUFFER_SIZE.
No official fixes yet, but I've attached a patch below that fixes these
(well, untested but compiles). I've limited the input buffer size to
only half of BIG_BUFFER_SIZE, that should fix most of the potential
problems.
ircii 20020912
--------------
The code was much better than I expected. snprintf() was used
everywhere, only problem that I found was a few my_strcat() calls:
1) "PRIVMSG a a*4080^ASED^A" or "^AUTC 1^A" overflows ctcp_buffer, but
not by much and there doesn't seem to be anything important right after
it
2) cannot_join_channel() allows writing one of 5 predefined texts past
buffer in stack. I was able to modify %ebp by with it, but couldn't get
anything useful out of it. It just crashed while trying to read memory.
I didn't try too hard though, so exploiting might be possible with
other ways.
3) Statusbar drawing has several buffer overflows. Usually it gets
truncated to screen width, but a few control characters aren't counted,
so we can stuff the buffer nearly full of it.
Next thing that happens is that the last character is filled until the
screen width is full. That alone can overflow the buffer. Then it
appends \017 + \0 without overflow checks.
status_make_printable() is called next. It has a check for pos <
BIG_BUFFER_SIZE, but that can still overflow buffer by two bytes (our
control char and \026) if last character is one the control chars.
I got this far with exploiting:
print SOCKET ":i 001 ".("\002"x3880).("X"x40).("\002"x140)." :\n";
Program received signal SIGSEGV, Segmentation fault.
0x08079f8a in make_status (window=0x58585858)
at /home/cras/src/ircii-20020912/source/status.c:803
803 if (window->status_line[k] && (SG == -1))
I think you'd get a real exploit out of that by properly setting the
local variables.
4) Some of the other my_strcat() calls may overflow buffer as well (eg.
create_server_list()), but they can't be exploited by (a single)
server.
ircii-20030313 fixes these.
EPIC4 1.1.7.20020907
--------------------
Note that this is an ALPHA version, and the author strongly recommends
not to use it. However at least Debian is still distributing it instead
of 1.0.1 (which is why I audited that - I just did "apt-get source
epic4").
It contained one user (eg. regular PRIVMSG) exploitable heap buffer
overflow if mangle_inbound setting contained ANSI. Because the
overflowable bytes were limited to only few specific characters, I'm
not sure if it's more than a remote crash.
It also contained the same two problems as 1.0.1 below, and a few more
potential ones.
20030314 snapshot should fix these.
EPIC4 1.0.1
-----------
This is the PRODUCTION release which you should be using.
1) EPIC has grown max. input line of server from the old 4096 to 8192,
but without growing BIG_SERVER_BUFFER from 4096. There's at least one
place where you can overflow it:
---
void userhost_cmd_returned (UserhostItem *stuff, char *nick, char *text)
{
char args[BIG_BUFFER_SIZE + 1];
/* This should be safe, though its playing it fast and loose */
strcpy(args, stuff->nick ? stuff->nick : empty_string);
strcat(args, stuff->oper ? " + " : " - ");
strcat(args, stuff->away ? "+ " : "- ");
strcat(args, stuff->user ? stuff->user : empty_string);
strcat(args, space);
strcat(args, stuff->host ? stuff->host : empty_string);
parse_line(NULL, text, args, 0, 0);
}
---
Exploiting that requires that the client calls /USERHOST nick -CMD
<command>, but I think that's quite widely used in EPIC scripts.
2) Statusbar writing isn't fully safe:
---
/*
* XXXXX This is a bletcherous hack.
* If i knew what was good for me id not do this.
*/
else if (*ptr == 9) /* TAB */
{
fillchar[0] = ' ';
fillchar[1] = 0;
do
*cp++ = ' ';
while (++(*prc) % 8);
ptr++;
}
---
Giving TAB at the end of nearly full buffer would overflow it. Normally
EPIC doesn't allow buffer to get wider than screen, but we can use
several control characters to fill the buffer without consuming screen
width.
Now, the only problem is how do you get large enough string into
statusbar. I didn't find any way to do it automatically by default. %C
is largest at 512 bytes. strip_ansi() can grow the string, but we'd
still need 2700 bytes or so. Some user-defined statusbar variables
could be used though, again requiring some script that uses them.
NOTE: I did only a quick check to see if the problems I found from
ALPHA version were still there. There's quite a lot of changes between
these versions, so there may be more and I'm too busy now to go through
it.
1.0.2 will not be released to fix server based exploits. 1.1.x series
intends to fully fix it, so in the mean time just don't connect to
untrusted servers. Growing BIG_BUFFER_SIZE to 8192 (and maybe a few
hundred bytes larger) should make you pretty safe though.
XChat 2.0.1
-----------
This isn't ircII-based, but I thought I'd check it as well.
Nothing usable found. A few minor potential buffer overflows here and
there, requiring conditions that currently don't exist. One thing I
don't understand is why does it bother playing around with unsafe
buffers while it could use GLIB's GStrings just as easily? This bothers
me a bit:
---
char outbuf[4096];
char pdibuf_static[522]; /* 1 line can potentially be 512*6 in utf8 */
---
outbuf is commonly treated as "large enough" to contain most of pdibuf.
This gives it around 1024 bytes of extra space. Is it enough? Seems to
be currently, but all you need for an overflow is to get two different
non-truncated server given strings written into it.
SOLUTION
BitchX Patch
------------
diff -ru BitchX-old/source/banlist.c BitchX/source/banlist.c
--- BitchX-old/source/banlist.c 2002-02-28 06:22:46.000000000 +0200
+++ BitchX/source/banlist.c 2003-03-13 20:09:01.000000000 +0200
@@ -277,30 +277,30 @@
case 7:
if (ip)
{
- sprintf(banstr, "*!*@%s", cluster(ip));
+ snprintf(banstr, sizeof(banstr), "*!*@%s", cluster(ip));
break;
}
case 2: /* Better */
- sprintf(banstr, "*!*%s@%s", t1, cluster(host));
+ snprintf(banstr, sizeof(banstr), "*!*%s@%s", t1, cluster(host));
break;
case 3: /* Host */
- sprintf(banstr, "*!*@%s", host);
+ snprintf(banstr, sizeof(banstr), "*!*@%s", host);
break;
case 4: /* Domain */
- sprintf(banstr, "*!*@*%s", strrchr(host, '.'));
+ snprintf(banstr, sizeof(banstr), "*!*@*%s", strrchr(host, '.'));
break;
case 5: /* User */
- sprintf(banstr, "*!%s@%s", t, cluster(host));
+ snprintf(banstr, sizeof(banstr), "*!%s@%s", t, cluster(host));
break;
case 6: /* Screw */
malloc_sprintf(&tmpstr, "*!*%s@%s", t1, host);
- strcpy(banstr, screw(tmpstr));
+ strmcpy(banstr, screw(tmpstr), sizeof(banstr)-1);
new_free(&tmpstr);
break;
case 1: /* Normal */
default:
{
- sprintf(banstr, "%s!*%s@%s", nick, t1, host);
+ snprintf(banstr, sizeof(banstr), "%s!*%s@%s", nick, t1, host);
break;
}
}
diff -ru BitchX-old/source/ctcp.c BitchX/source/ctcp.c
--- BitchX-old/source/ctcp.c 2002-02-28 06:22:47.000000000 +0200
+++ BitchX/source/ctcp.c 2003-03-13 19:59:35.000000000 +0200
@@ -1482,6 +1482,7 @@
*putbuf2;
int len;
len = IRCD_BUFFER_SIZE - (12 + strlen(to));
+ if (len <= 2) return;
putbuf2 = alloca(len);
if (format)
diff -ru BitchX-old/source/misc.c BitchX/source/misc.c
--- BitchX-old/source/misc.c 2002-03-24 11:31:07.000000000 +0200
+++ BitchX/source/misc.c 2003-03-13 20:02:13.000000000 +0200
@@ -3121,19 +3121,19 @@
{
if (*hostname == '~')
hostname++;
- strcpy(result, hostname);
+ strmcpy(result, hostname, sizeof(result)-1);
*strchr(result, '@') = '\0';
if (strlen(result) > 9)
{
result[8] = '*';
result[9] = '\0';
}
- strcat(result, "@");
+ strmcat(result, "@", sizeof(result)-1);
if (!(hostname = strchr(hostname, '@')))
return NULL;
hostname++;
}
- strcpy(host, hostname);
+ strmcpy(host, hostname, sizeof(host)-1);
if (*host && isdigit(*(host + strlen(host) - 1)))
{
@@ -3154,8 +3154,8 @@
for (i = 0; i < count; i++)
tmp = strchr(tmp, '.') + 1;
*tmp = '\0';
- strcat(result, host);
- strcat(result, "*");
+ strmcat(result, host, sizeof(result)-1);
+ strmcat(result, "*", sizeof(result)-1);
}
else
{
@@ -3177,10 +3177,10 @@
else
return (char *) NULL;
}
- strcat(result, "*");
+ strmcat(result, "*", sizeof(result)-1);
if (my_stricmp(host, temphost))
- strcat(result, ".");
- strcat(result, host);
+ strmcat(result, ".", sizeof(result)-1);
+ strmcat(result, host, sizeof(result)-1);
}
return result;
}
diff -ru BitchX-old/source/names.c BitchX/source/names.c
--- BitchX-old/source/names.c 2002-03-25 22:47:30.000000000 +0200
+++ BitchX/source/names.c 2003-03-13 20:10:26.000000000 +0200
@@ -572,7 +572,7 @@
*nmodes = 0;
*nargs = 0;
- for (; *modes; modes++)
+ for (; *modes && strlen(nmodes) < sizeof(nmodes)-2; modes++)
{
isbanned = isopped = isvoiced = 0;
switch (*modes)
@@ -742,7 +742,7 @@
/* modes which can be done multiple times are added here */
- for (tucm = ucm; tucm; tucm = tucm->next)
+ for (tucm = ucm; tucm && strlen(nmodes) < sizeof(nmodes)-2; tucm = tucm->next)
{
if (tucm->o_ed)
{
diff -ru BitchX-old/source/notice.c BitchX/source/notice.c
--- BitchX-old/source/notice.c 2002-02-28 06:22:50.000000000 +0200
+++ BitchX/source/notice.c 2003-03-13 20:07:39.000000000 +0200
@@ -422,10 +422,10 @@
{
char *q = strchr(line, ':');
char *port = empty_string;
- int conn = !strncmp(line+7, "connect", 7) ? 1 : 0;
+ int conn = strlen(line) > 7 && !strncmp(line+7, "connect", 7) ? 1 : 0;
int dalnet = 0, ircnet = 0;
- if (*(line+18) == ':')
+ if (strlen(line) > 18 && *(line+18) == ':')
q = NULL;
else
dalnet = (q == NULL);
@@ -462,7 +462,7 @@
else sscanf(p, "%s was %s from %s", for_, fr, temp);
q = p;
- sprintf(q, "%s@%s", fr, temp);
+ snprintf(q, strlen(q)+1, "%s@%s", fr, temp);
if (!conn)
{
port = strstr(temp2, "reason:");
diff -ru BitchX-old/source/server.c BitchX/source/server.c
--- BitchX-old/source/server.c 2002-03-25 07:21:24.000000000 +0200
+++ BitchX/source/server.c 2003-03-13 20:10:00.000000000 +0200
@@ -474,11 +474,11 @@
}
else
#endif
- junk = dgets(bufptr, des, 1, BIG_BUFFER_SIZE, server_list[i].ssl_fd);
+ junk = dgets(bufptr, des, 1, BIG_BUFFER_SIZE/2, server_list[i].ssl_fd);
}
else
#endif
- junk = dgets(bufptr, des, 1, BIG_BUFFER_SIZE, NULL);
+ junk = dgets(bufptr, des, 1, BIG_BUFFER_SIZE/2, NULL);
}
switch (junk)
{
@@ -1741,7 +1741,7 @@
default:
if (FD_ISSET(des, &rd))
{
- if (!dgets(buffer, des, 0, BIG_BUFFER_SIZE, NULL))
+ if (!dgets(buffer, des, 0, BIG_BUFFER_SIZE/2, NULL))
flushing = 0;
}
break;
@@ -1751,7 +1751,7 @@
FD_ZERO(&rd);
FD_SET(des, &rd);
if (new_select(&rd, NULL, &timeout) > 0)
- dgets(buffer, des, 1, BIG_BUFFER_SIZE, NULL);
+ dgets(buffer, des, 1, BIG_BUFFER_SIZE/2, NULL);
}