Fix a use-after-free in job_chldtrap

The sh_trap() call might unset or modify the trap handler. Thus, the string stored in "trap" might be freed between two iterations. See also: fc53766
l0stman · Nov 8, 2020 · 2033375 · 2033375
1 parent d921a93
commit 2033375
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 5 deletions.
diff --git a/src/cmd/ksh93/include/jobs.h b/src/cmd/ksh93/include/jobs.h
@@ -151,7 +151,7 @@ extern bool job_wait(pid_t);
 extern int job_post(Shell_t *, pid_t, pid_t);
 extern void *job_subsave(void);
 extern void job_subrestore(Shell_t *, void *);
-extern void job_chldtrap(Shell_t *, const char *, int);
+extern void job_chldtrap(Shell_t *, int);
 #ifdef JOBS
 extern void job_init(Shell_t *, int);
 extern int job_close(Shell_t *);

diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c
@@ -402,7 +402,7 @@ void sh_chktrap(Shell_t *shp) {
         if ((shp->sigflag[sig] & SH_SIGTRAP) || (shp->siginfo && shp->siginfo[sig])) {
             shp->sigflag[sig] &= ~SH_SIGTRAP;
             if (sig == SIGCHLD) {
-                job_chldtrap(shp, shp->st.trapcom[SIGCHLD], 1);
+                job_chldtrap(shp, 1);
                 continue;
             }
             trap = shp->st.trapcom[sig];

diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c
@@ -206,7 +206,7 @@ static_fn void setsiginfo(Shell_t *shp, siginfo_t *info, struct process *pw) {
     sh_setsiginfo(info);
 }
 
-void job_chldtrap(Shell_t *shp, const char *trap, int lock) {
+void job_chldtrap(Shell_t *shp, int lock) {
     UNUSED(lock);
     struct process *pw, *pwnext;
     pid_t bckpid;
@@ -225,7 +225,9 @@ void job_chldtrap(Shell_t *shp, const char *trap, int lock) {
         shp->bckpid = pw->p_pid;
         shp->savexit = pw->p_exit;
         if (pw->p_flag & P_SIGNALLED) shp->savexit |= SH_EXITSIG;
-        sh_trap(shp, trap, 0);
+        // We need to reread the handler each time since sh_trap()
+        // might unset or change it.
+        sh_trap(shp, shp->st.trapcom[SIGCHLD], 0);
         if (!sorc) {
             job.numbjob--;
             if (pw->p_pid == bckpid) job_unpost(shp, pw, 0);
@@ -486,7 +488,7 @@ bool job_reap(int sig) {
     if (!sig && (shp->trapnote & SH_SIGTRAP)) {
         int c = job.in_critical;
         job.in_critical = 0;
-        job_chldtrap(shp, shp->st.trapcom[SIGCHLD], 1);
+        job_chldtrap(shp, 1);
         job.in_critical = c;
     }
     return nochild;