Improve child error handling

Always set SIGCHLD handler to default, even if the caller of vipw has
set SIGCHLD to ignore. If SIGCHLD is ignored no zombie processes would
be created, which in turn could mean that kill is called with an already
recycled pid.

Proof of Concept:

1. Compile nochld:
 --
 #include <signal.h>
 #include <unistd.h>
 int main(void) {
 char *argv[] = { "vipw", NULL };
 signal(SIGCHLD, SIG_IGN);
 execvp("vipw", argv);
 return 1;
 }
 --
2. Run nochld
3. Suspend child vi, which suspends vipw too:
`kill -STOP childpid`
4. Kill vi:
`kill -9 childpid`
5. You can see with ps that childpid is no zombie but disappeared
6. Bring vipw back into foreground
`fg`

The kill call sends SIGCONT to "childpid" which in turn could have been
already recycled for another process.

This is definitely not a vulnerability. It would take super user
operations, at which point an attacker would have already elevated
permissions.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

diff --git a/src/vipw.c b/src/vipw.c
index 94185c3df95cc6f8ec25c4c4c74213f210cc77f4..1a69ef285ce2964cac4510485baaf06def827441 100644 (file)
--- a/src/vipw.c
+++ b/src/vipw.c
@@ -349,6 +349,9 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
                sigprocmask(SIG_BLOCK, &mask, &omask);
        }
 
+       /* set SIGCHLD to default for waitpid */
+       signal(SIGCHLD, SIG_DFL);
+
        for (;;) {
                pid = waitpid (pid, &status, WUNTRACED);
                if ((pid != -1) && (WIFSTOPPED (status) != 0)) {