Iljas Blag

a.k.a NetBSD blowing up on a trivial ioctl fuzzer.

A while ago I was reading the gdb manual. Apparently it reads the .gdbinit file from your homedirectory AND from the current working directory:
“2.1.3 What gdb does during startup
Here’s the description of what gdb does during session startup:
1. Sets up the command interpreter as specified by the command line (see Section 2.1.2
[Mode Options], page 13).
2. Reads the init file (if any) in your home directory1 and executes all the commands in
that file.
3. Processes command line options and operands.
4. Reads and executes the commands from init file (if any) in the current working directory.
This is only done if the current directory is different from your home directory.
Thus, you can have more than one init file, one generic in your home directory, and
another, specific to the program you are debugging, in the directory where you invoke
gdb.”

Anyone besides me that thinks this is a dumb idea ?
Btw, in case you didnt know, it’s possible to have shell commands in your .gdbinit file.
Anyways, next time I’m on a shellserver you can bet I’ll have an evil .gdbinit file in /tmp.

Same httpd as I blogged about earlier:
strlcpy(orig, params, MYBUFSIZ);
size = strlen(orig);

if (size < NI_MAXHOST &&
sscanf(params, “http://%[^/]%c”, http_host, &ch) == 2 &&
ch == ‘/’)

params can be up to 4096 bytes and is usercontrolled, MYBUFSIZ is 1024 and NI_MAXHOST is 1025 !!

This is the funniest bounds check fuckup i’ve seen in a while.

I rest my case.

I was going over some code in some httpd when I saw the following:
strftime(modified, sizeof(modified), “%a, %d %b %Y %H:%M:%S GMT”, gmtime(&modtime));
where modtime is under a users control. modfied is a very small buffer.

not knowing exactly all the features of strftime I looked in the manpage and read the following:
“The strftime() function returns the number of characters placed in the array s, not including the terminating null byte, provided the string, including the terminating null byte, fits. Otherwise, it returns 0, and the contents of the array is undefined. (Thus at least since libc 4.4.4; very old versions of libc, such as libc 4.4.1, would return max if the array was too small.)”

Obviously that line of code is wrong. as it turns out “modified” gets written back to the user, so this piece of code could leak information, or possible cause a segfault when an unmapped page gets hit (since it’s assumed it’s a 0-terminated string).

is it really that hard to always and consistently check return values ?

So I was reading some rfc’s and apparently it’s forbidden to send chain letters via electronic mail. It’s in the rfc, so it must be true. rfc1855:
“Never send chain letters via electronic mail. Chain lettersare forbidden on the Internet. Your network privileges will be revoked. Notify your local system administrator if your ever receive one.”

also, I just saw that there are “scream awards” on tv, wtf ??? people are giving away awards for everything these days. If only there were slacker awards, I’d win every single one of them.

I recently saw this movie “stormbreaker” it totally sux !! don’t spend money on it, hell don’t even spend bandwidth on it, it’s not worth it.

I guess it’s time for another this-api-is-so-broken blogpost. fclose() as most know is used to close a stream that was obtained from fopen(), fdopen(), popen(), … . However, almost noone actually bothers checking the return value of fclose. According to the manpage it CAN fail, and hence you should adleast see if it does or not. fclose() is however broken by design imo. The maanpage states the following:
“Upon successful completion 0 is returned. Otherwise, EOF is returned and the global variable errno is set to indicate the error. In either case any further access (including another call to fclose()) to the stream results in undefined behaviour.”
It means that should you check the return value of fclose() and detect that it failed, then there is nothing that you can do about it (hm, well, you could try, but that might lead to heap corruption, something you probably want to avoid), in other words, you’re doomed if you do and you’re doomed if you don’t.
I guess the only thing you could do (if you’re trying to write correct code) is to basicly abort immediatly if you detect fclose() failed.

This brings me to close() which isn’t as busted as fclose() but suffers from the same usage problem. Almost noone ever checks it’s return value, and yet, according to POSIX it can fail. The manpage says the following about it:
“ERRORS

EBADF
fd isn’t a valid open file descriptor.
EINTR
The close() call was interrupted by a signal.
EIO
An I/O error occurred.”

so it could for example get interrupted by a signal, and leave the fd open. The linux manpage goes on and states:
“Not checking the return value of close is a common but nevertheless serious programming error. It is quite possible that errors on a previous write(2) operation are first reported at the final close(). Not checking the return value when closing the file may lead to silent loss of data. This can especially be observed with NFS and with disk quota.”

People should seriously start checking the return value of calls like close(). Not doing so could lead to fd leaks, and placed in the right context those can be a huge security issue.

It so does !
Wtf were those guys thinking ? Most of the stuff in there is redundant ! g_malloc() ?!? Hello ??? this shit is useless ! not to mention that using glib makes your code look ugly.

glib also has it’s own “basic types”
you know, gint, guint, gboolean, gconstpointer ( _YES_, gconstpointer, coz that really makes it look nicer , also, see http://www.kuro5hin.org/story/2003/8/12/231143/644 ).

g_strdup_printf(), wtf is wrong with asprintf() ?

I could go on and on, but I won’t.

Some claim that it’s usefull for portable code, I disagree. We all know portable code looks ugly from time to time, there is no need to obfuscate it even further !

Also, I think they missed a useless function:
g_lib_sucks_big_hairy_monkey_balls_like_sweet_chocolate_candy()

A while ago I was reading some glib code (and I’m still sort of sane, I wonder how I pulled that off) and spotted some interesting code:

#define g_new(struct_type, n_structs) \
((struct_type *) g_malloc (((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))
#define g_new0(struct_type, n_structs) \
((struct_type *) g_malloc0 (((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))
#define g_renew(struct_type, mem, n_structs) \
((struct_type *) g_realloc ((mem), ((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))

#define g_try_new(struct_type, n_structs) \
((struct_type *) g_try_malloc (((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))
#define g_try_new0(struct_type, n_structs) \
((struct_type *) g_try_malloc0 (((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))
#define g_try_renew(struct_type, mem, n_structs) \
((struct_type *) g_try_realloc ((mem), ((gsize) sizeof (struct_type)) * ((gsize) (n_structs))))

can you say integer overflow ? I did report this to the glib people, never got a response (the copypast is from the latest version)

ofcourse they also have alloca wrappers:

#define g_alloca(size) alloca (size)
#define g_newa(struct_type, n_structs) ((struct_type*) g_alloca (sizeof (struct_type) * (gsize) (n_structs)))

Because alloca() is so secure !!1!. yea, g_newa() also has an overflow, I’m shocked I tell you, shocked !

I can’t take people who use glib serious, I tried, I failed. If you’re a programmer and feel you must use glib then maybe it’s time to look for a new profession.

it seems a bug I reported in proftpd got mistaken with another bug that Evgeny Legerov found ages ago. A couple of weeks ago I was browsing thru the proftpd code and stumbled on some issues, one of them is the so called “CommandBufferSize thing”. This one is an off-by-two underflow. the bug is located in the following piece of code:

static void cmd_loop(server_rec *server, conn_t *c) {
static long cmd_buf_size = -1;
config_rec *id = NULL;
char buf[PR_TUNABLE_BUFFER_SIZE] = {”};
…
if (pr_netio_telnet_gets(buf, sizeof(buf)-1, session.c->instrm,
session.c->outstrm) == NULL) {
…
}
if (cmd_buf_size == -1) {
long *buf_size = get_param_ptr(main_server->conf,
“CommandBufferSize”, FALSE);

if (buf_size == NULL || *buf_size sizeof(buf)) {
pr_log_pri(PR_LOG_WARNING, “Invalid CommandBufferSize size given. ”
“Resetting to 512.”);
cmd_buf_size = 512;
}
}

buf[cmd_buf_size - 1] = ”;
….

if CommandBufferSize is set, and it’s set to something reasonable (go figure) it causes an off-by-two underflow. (for those who don’t get it, cmd_buf_size remains -1 and hence the following happens buf[-1 -1] = ”; and for those who failed elementary school math’s that’s buf[-2] = ”;).
I also reported some other bugs in proftpd, including some readlink() off-by-one’s in the ls code (3 of them to be exact), one of them I think has potential:

static int nlstdir(cmd_rec *cmd, const char *dir) {
char **list, *p, *f,
file[PR_TUNABLE_PATH_MAX + 1] = {”};
…
if ((i = pr_fsio_readlink(p, file, sizeof(file))) > 0) {
file[i] = ”;
f = file;
…
}

I blogged about readlink abuse before, but for those who don’t see it, readlink() returns how many bytes it copied and doesn’t 0-terminate. so if it copied sizeof(file) bytes, that’s what it’ll return. So sizeof(file) gets used as an index to file, and hence causes an off-by-one. Depending on the compiler being used there’s a possibility you smash right into the saved frame pointer.

I also found a bunch of other off-by-a-one’s and off-by-a-few’s (like their “secure” strcpy code I blogged about earlier) but exploitation of them seems unlikely.

The carefull reader might have seen that all the crap I’m talking about are off-by-one’s and off- by-a-few’s and they would be right. ProFTPD is off-by-one paradise !!

/* just in case you were wondering, this is why DCC is gay */
static void irc_dccsend_send_read(gpointer data, int source, GaimInputCondition cond)
{
GaimXfer *xfer = data;
struct irc_xfer_send_data *xd = xfer->data;
char *buffer[16];
int len;

len = read(source, buffer, sizeof(buffer));

In case you were wondering this is why gaim is gay.

if (xd->rxlen) {
unsigned char *tmp = g_memdup(xd->rxqueue + 4, xd->rxlen);
g_free(xd->rxqueue);
xd->rxqueue = tmp;
}

because advancing a pointer by 4 is too much of a hassle.

} else if (!strncmp(cur, “PING “, 5)) {
if (notice) { /* reply */
/* TODO: Should this read in the timestamp as a double? */
sscanf(cur, “PING %lu”, &timestamp);
gc = gaim_account_get_connection(irc->account);
if (!gc)
return NULL;
buf = g_strdup_printf(_(”Reply time from %s: %lu seconds”), from, time(NULL) – timestamp);
gaim_notify_info(gc, _(”PONG”), _(”CTCP PING reply”), buf);
g_free(buf);
return NULL;
}

yea ctcp ping infoleaks !!1!

/* Note that this is NOT correct w.r.t. multiple CTCPs in one
* message and low-level quoting … but if you want that crap,
* use a real IRC client. */

That sure shows you’re confident in your own code.

omfg, that code is unbelievable.

I just remembered. I was at hack.lu a couple of weeks ago, and there was a guy giving a talk about how to cause an unlink() without calling free() (in heap exploitation) He worked out some situation where this is possible with just calling malloc() in the solaris and aix heap allocators. During the QA a buddy of mine asked “Have you tried this with a modern os ?”. I laughed my ass of with that question (for those who don’t get it, he’s implying that aix and solaris are not modern operating systems) but apparently I was the only one in the entire room that though it was funny.