From bee2e1c13b20b58412dacbc6d2d1eaf9bcdde138 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Fri, 19 Aug 2022 13:56:35 -0700 Subject: [PATCH] funct(): Fix another use after free bug (re: f24040ee, 69d37d5e) (#519) The ASan crash in basic.sh when sourcing multiple files is caused by a bug that is similar to the crash fixed in f24040ee. This is the trace for the regression test crash (note that in order to see the trace, the 2>/dev/null redirect must be disabled): ==1899388==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000005b0 at pc 0x55a5e3f9432a bp 0x7ffeb91ea110 sp 0x7ffeb91ea100 WRITE of size 8 at 0x6150000005b0 thread T0 #0 0x55a5e3f94329 in funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:967 #1 0x55a5e3f96f77 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1349 #2 0x55a5e3f90c9f in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:642 #3 0x55a5e3f90ac1 in list /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:613 #4 0x55a5e3f90845 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:561 #5 0x55a5e3f909e0 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:586 #6 0x55a5e3f8fd5e in sh_parse /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:438 #7 0x55a5e3fc43c1 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:635 #8 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #9 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #10 0x55a5e3fd01d4 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1932 #11 0x55a5e3fc4544 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:651 #12 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #13 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #14 0x55a5e3ecc1cd in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604 #15 0x55a5e3ec9e7f in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369 #16 0x55a5e3ec801d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41 #17 0x7f637b4db2cf (/usr/lib/libc.so.6+0x232cf) #18 0x7f637b4db389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) #19 0x55a5e3ec7f24 in _start ../sysdeps/x86_64/start.S:115 Code in question: https://github.com/ksh93/ksh/blob/8d57369b0cb39074437dd82924b604155e30e1e0/src/cmd/ksh93/sh/parse.c#L963-L968 To avoid any more similar crashes, all of the fixes introduced in 69d37d5e that set slp->slptr to null have been improved with the fix in f24040ee. --- src/cmd/ksh93/sh/name.c | 3 ++- src/cmd/ksh93/sh/parse.c | 3 ++- src/cmd/ksh93/sh/xec.c | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index c3cd0e66c..55387a66b 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -2466,8 +2466,9 @@ void _nv_unset(register Namval_t *np,int flags) } if(slp->slptr) { - stakdelete(slp->slptr); + Stak_t *sp = slp->slptr; slp->slptr = NIL(Stak_t*); + stakdelete(sp); } free((void*)np->nvalue.ip); np->nvalue.ip = 0; diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c index 070bd2df1..5e219de45 100644 --- a/src/cmd/ksh93/sh/parse.c +++ b/src/cmd/ksh93/sh/parse.c @@ -963,8 +963,9 @@ static Shnode_t *funct(Lex_t *lexp) if(slp && slp->slptr) { sh.st.staklist = slp->slnext; - stakdelete(slp->slptr); + Stak_t *slptr_save = slp->slptr; slp->slptr = NIL(Stak_t*); + stakdelete(slptr_save); } siglongjmp(*sh.jmplist,jmpval); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 5f240ce34..879adb77a 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1411,8 +1411,9 @@ int sh_exec(register const Shnode_t *t, int flags) sh_funstaks(slp->slchild,-1); if(slp->slptr) { - stakdelete(slp->slptr); + Stak_t *sp = slp->slptr; slp->slptr = NIL(Stak_t*); + stakdelete(sp); } if(jmpval > SH_JMPFUN || (io && jmpval > SH_JMPIO)) siglongjmp(*sh.jmplist,jmpval); @@ -2462,8 +2463,9 @@ int sh_exec(register const Shnode_t *t, int flags) sh_funstaks(slp->slchild,-1); if(slp->slptr) { - stakdelete(slp->slptr); + Stak_t *sp = slp->slptr; slp->slptr = NIL(Stak_t*); + stakdelete(sp); } if(rp->sdict) {