Re: Nmap doesn't register signal handlers

[Solar Designer <solar () openwall com>]

On Wed, Aug 12, 2009 at 12:52:21AM +0300, ithilgore wrote:
> void sigdie(int signo) {
>   int abt = 0;
>
>   fflush(stdout);

This call, as well as most or even all other calls made from this signal
handler, are unsafe.  There are only a handful of functions that may be
safely used in a signal handler; most are unsafe,  Any/all use of stdio
is unsafe.

Please see:

https://www.securecoding.cert.org/confluence/display/seccode/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers
http://www.openbsd.org/cgi-bin/man.cgi?query=signal

>   switch(signo) {
>   case SIGINT:
>     error("caught SIGINT signal, cleaning up");

Perhaps error() does risky things, too.

>   fflush(stderr);

Unsafe, unneeded (stderr is normally not buffered anyway).

>   log_close(LOG_MACHINE|LOG_NORMAL|LOG_SKID);

Likely unsafe (I did not check that function).

>   if (abt) abort();

abort() flushes and closes stdio streams, hence it is unsafe.

>   exit(1);

Same as above, plus invokes atexit handlers, hence unsafe.

Sure, a lot of programs do stuff like this (and segfault on it once in a
while), but if you're working on this code currently it may be a good
time to fix it.  Given the purpose of the signal handler above, a proper
fix, while preserving the functionality, is non-trivial.  Maybe you'll
have to sacrifice some of the functionality.

One idea is to set a global "volatile sig_atomic_t" flag variable on
first occurrence of a signal, have the program's various loops check for
the flag and invoke a function when the flag turns out to be set.  This
is what JtR does (I have "volatile int" in there, though - for ancient
systems - but it's time to be switching to sig_atomic_t).  You may
search these source files for "event_" for an example:

http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/john/john/src/signals.c
http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/john/john/src/cracker.c

Another idea is to use write(2) (with a loop around it) and _exit(2) -
just the direct syscalls - to print an error message and terminate the
program right from the signal handler.

An even better idea is to use a combination of both approaches - merely
set a flag on the first occurrence of a signal (and let the main program
handle it), but if the signal arrives again (perhaps because the main
program failed to handle the flag in time), resort to write/_exit.  This
is what JtR does in sig_handle_abort().  So under normal conditions, one
Ctrl-C keypress results in a not-so-instant but clean shutdown.  When
the program is "locked up" or when the user is being impatient, a second
Ctrl-C keypress results in an immediate shutdown.

Here's the relevant code (a bit simplified - I removed some detail
irrelevant in this context):

static void sig_handle_abort(int signum)
{
        int saved_errno = errno;

        if (event_abort) {
                write_loop(2, "Session aborted\n", 16);
                _exit(1);
        }

        event_abort = event_pending = 1;

        write_loop(2, "Wait...\r", 8);

        sig_install_abort();

        errno = saved_errno;
}

Finally, I recommend against trying to catch SIGSEGV and act on it.
This may turn some segfault-only bugs into arbitrary code execution
vulnerabilities.  It may also make debugging harder, not easier.

I hope this helps.

Alexander

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org