#28 log_info, log_debug, etc. not safe

[bas van klinkenberg]

Hi,

i'm using libcsoap1.1.

It is possible to crash nanohttp by using large arguments in the va_list for log_info, log_debug, etc.

This is due to the fact that _log_write (in nanohttp-logging.c) uses static length buffers on the stack (char buffer[1054]), and uses vsprintf to write to those buffers. vsprintf does not check the length, so if the va_list contains large arguments, it overflows the buffer, causing a crash.

The problem can be circumvented by using vsnprintf instead, to limit the amount of characters written to the buffer.

Or even nicer would be to use vasprintf which mallocs the buffer it needs, and fall back on the static buffer with vsnprintf if running out of memory.

Here is a patch (ignore the revision number - it belongs to our project repository):

Index: nanohttp/nanohttp-logging.c

--- nanohttp/nanohttp-logging.c (revision 5900)
+++ nanohttp/nanohttp-logging.c (working copy)
@@ -118,6 +118,8 @@
{
char buffer[1054];
char buffer2[1054];
+ char *vabuffer = NULL;
+ char *printbuffer;
FILE *f;

if (level < loglevel)
@@ -131,10 +133,17 @@
sprintf(buffer, "*%s*:(%ld) [%s] %s\n",
prefix, pthread_self(), func, format);
#endif
- vsprintf(buffer2, buffer, ap);
+ /* try to print the whole string with vasprintf. This won't work if we want to log an
+ out of memory message, so in that case, use vsnsprintf with a fixed length buffer */
+ if (vasprintf(&vabuffer, buffer, ap) == -1) {
+ vsnprintf(buffer2, 1054, buffer, ap);
+ printbuffer = buffer2;
+ } else {
+ printbuffer = vabuffer;
+ }
if (!log_background)
{
- printf(buffer2);
+ printf(printbuffer);
fflush(stdout);
}

@@ -145,11 +154,13 @@
f = fopen(hlog_get_file(), "w");
if (f)
{
- fprintf(f, buffer2);
+ fprintf(f, printbuffer);
fflush(f);
fclose(f);
}
}
+ if (vabuffer)
+ free(vabuffer);
}
}