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>
This commit is contained in:
Tobias Stoeckmann 2021-11-14 12:01:32 +01:00
parent 387da46d73
commit 624d57c08c

View File

@ -349,6 +349,9 @@ vipwedit (const char *file, int (*file_lock) (void), int (*file_unlock) (void))
sigprocmask(SIG_BLOCK, &mask, &omask); sigprocmask(SIG_BLOCK, &mask, &omask);
} }
/* set SIGCHLD to default for waitpid */
signal(SIGCHLD, SIG_DFL);
for (;;) { for (;;) {
pid = waitpid (pid, &status, WUNTRACED); pid = waitpid (pid, &status, WUNTRACED);
if ((pid != -1) && (WIFSTOPPED (status) != 0)) { if ((pid != -1) && (WIFSTOPPED (status) != 0)) {