From 624d57c08caceed306212d24c2147f6273f3fc4b Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 14 Nov 2021 12:01:32 +0100 Subject: [PATCH] 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 #include 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 --- src/vipw.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vipw.c b/src/vipw.c index 94185c3d..1a69ef28 100644 --- 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)) {