Core dump in svcstatus.c (historylog) with empty TIMEBUF
list Jeremy Laidman
Folks
I can crash svcstatus, causing a core dump, by doing a historylog
query with an empty TIMEBUF value (or setting it to things like
"%00"). It's indirectly caused by a successful stat() on a directory
and then assuming that it's a file, but subsequent reads from the
directory causes chaos with pointers. My fix is to catch the special
case of a directory. I suspect the log file handling code that comes
after stat() could be made a bit more suspicious/robust, but I don't
have sufficient skill to do this task myself.
Here's my patch:
--- web/svcstatus.c.orig 2011-05-06 15:36:11.000000000 +1000
+++ web/svcstatus.c 2011-05-09 15:30:18.000000000 +1000
@@ -467,7 +467,7 @@
p = tstamp; while ((p = strchr(p, '_')) != NULL) *p = ' ';
sethostenv_histlog(tstamp);
- if ((stat(logfn, &st) == -1) || (st.st_size < 10)) {
+ if ((stat(logfn, &st) == -1) || (st.st_size < 10) ||
S_ISDIR(st.st_mode)) {
errormsg("Historical status log not available\n");
return 1;
}
Cheers
Jeremy
list Henrik Størner
▸
On 09-05-2011 07:34, Jeremy Laidman wrote:
I can crash svcstatus, causing a core dump, by doing a historylog query with an empty TIMEBUF value (or setting it to things like "%00"). It's indirectly caused by a successful stat() on a directory and then assuming that it's a file,
Yep, it shouldn't try to do that.
My fix is to catch the special case of a directory.
Better to check if it's a regular file (i.e. not a pipe, socket, ...) See attached. Regards, Henrik
Attachments (1)
list Jeremy Laidman
Yup, that's heaps better than mine - and that's why you're the programmer and not me! Awesome, thanks.
▸
On Mon, May 9, 2011 at 4:15 PM, Henrik Størner <user-ce4a2c883f75@xymon.invalid> wrote:Better to check if it's a regular file (i.e. not a pipe, socket, ...) See attached.