Package: restartd
Version: 0.2.2

When using restartd to monitor processes that fork a lot (apache, havp,
...), an incoherent behaviour happens. Typically syslog get filled with
lines of this type:

Mar 11 17:16:56 $HOSTNAME restartd:  19326 19328 19331 19334 19337 19340
19342 19345 19347 19349 19351 19353 19355 19357 19359 19362 19364 19366
19367 19369 19371 19373 19375 19380 19383 19385 19387 19389 19391 19393
19395 19397 19399 19402 19405 19407 19409 19411 19413 19415 19417 19419
19421 19423 19425 19427 19430 19432 19434 19436 19438 19440 19442 is NOT
running, executing ' 19399 19402 19405 19407 19409 19411 19413 19415
19417 19419 19421 19423 19425 19427 19430 19432 19434 19436 19438 19440
19442'

This seems to be related to a buffer overflow in restartd code. Overflow
could be exploited to crash restartd, or, although unlikely, call
arbitrary commands which names are PIDs, and present in restartd $PATH
(maybe code execution but I can't see how given the fact data is only
PIDs).

This can been seen on the line 
  executing ' 19399 19402 ...
as later, restartd uses this data as argument for a system(3) function call.

Relevant code is located in main.c file, line 246 to 257:

// Check process
for (i = 0; i < config_process_number; i++)
  {
    regcomp (regc, config_process[i].regexp, REG_EXTENDED
             || REG_ICASE);
    if (!regexec
        (regc, proc_cmdline_str, r_nmatch, r_match, 0))
      sprintf (config_process[i].processes, "%s %s",
               config_process[i].processes,
               procdir_dirent->d_name);
    regfree (regc);
  }

for a large number of running "monitored" process (which match regular
expression specified in configuration file), PIDs keep being appended to
config_process[i].processes, without any check. 'config_process_type'
struct has its 'processes' member limited to 256 bytes, as defined in
config.h file, line 44 to 54.

It seems this quick ugly fix works with the test setup I used
(4 monitored processes forking 100 times each).

if (snprintf (config_process[i].processes, 256, "%s %s",
        config_process[i].processes,
        procdir_dirent->d_name) >= 256 )
{
        config_process[i].processes[256-1] = '\0';
}

Sadfully this fix interferes with the /var/run/restartd status file
maintained by restartd as PIDs list gets truncated.

Note another sprintf use without error checking at main.c line 233


This has been tested on Debian GNU/Linux 5.0 (Lenny), kernel
2.6.26-1-686 and libc6 2.7-18 and reported on many other Debian Lenny
systems.

Attachment: signature.asc
Description: Digital signature

Reply via email to