From 69d37d5eaeba905400a7c2b71f9d6f36373b5cb7 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 14 Jun 2022 13:18:48 +0100 Subject: [PATCH] parse.c: fix use-after-free probs related to funstaks() When the funstaks() function deletes a stack, other code could still reference that stack's pointer, at least if a script's DEBUG trap changed the function context by assigning to ${.sh.level}. This crashed @ormaaj's funcname.ksh script in certain contexts, at least when run as a dot script or in a virtual subshell. This allows that script to run in all contexts by making funstaks(s) set the stack pointer to NULL after deleting the stack and making various other points in the code check for a null pointer before dereferencing it. This may not be the most elegant fix but (in my testing) it does work, even when compiling ksh with AddressSanitiser. Thanks to @JohnoKing for help researching this problem. Resolves: https://github.com/ksh93/ksh/issues/212 --- src/cmd/ksh93/sh/name.c | 6 +- src/cmd/ksh93/sh/parse.c | 15 +++-- src/cmd/ksh93/sh/xec.c | 18 ++++-- src/cmd/ksh93/tests/functions.sh | 95 ++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 9 deletions(-) diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index 9fb3c5776..f6aa65b1c 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -2501,7 +2501,11 @@ void _nv_unset(register Namval_t *np,int flags) } dtclose(rp->sdict); } - stakdelete(slp->slptr); + if(slp->slptr) + { + stakdelete(slp->slptr); + slp->slptr = NIL(Stak_t*); + } 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 1f8dede51..744e5b55e 100644 --- a/src/cmd/ksh93/sh/parse.c +++ b/src/cmd/ksh93/sh/parse.c @@ -527,10 +527,16 @@ void sh_funstaks(register struct slnod *slp,int flag) if(slp->slchild) sh_funstaks(slp->slchild,flag); slp = slp->slnext; - if(flag<=0) - stakdelete(slpold->slptr); - else - staklink(slpold->slptr); + if(slpold->slptr) + { + if(flag<=0) + { + stakdelete(slpold->slptr); + slpold->slptr = NIL(Stak_t*); + } + else + staklink(slpold->slptr); + } } } /* @@ -953,6 +959,7 @@ static Shnode_t *funct(Lex_t *lexp) { sh.st.staklist = slp->slnext; stakdelete(slp->slptr); + slp->slptr = NIL(Stak_t*); } siglongjmp(*sh.jmplist,jmpval); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 45db4bed6..9d362241a 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1421,7 +1421,8 @@ int sh_exec(register const Shnode_t *t, int flags) /* increase refcnt for unset */ slp = (struct slnod*)np->nvenv; sh_funstaks(slp->slchild,1); - staklink(slp->slptr); + if(slp->slptr) + staklink(slp->slptr); if(nq) { Namval_t *mp=0; @@ -1469,7 +1470,11 @@ int sh_exec(register const Shnode_t *t, int flags) if(nq) unset_instance(nq,&node,&nr,mode); sh_funstaks(slp->slchild,-1); - stakdelete(slp->slptr); + if(slp->slptr) + { + stakdelete(slp->slptr); + slp->slptr = NIL(Stak_t*); + } if(jmpval > SH_JMPFUN || (io && jmpval > SH_JMPIO)) siglongjmp(*sh.jmplist,jmpval); goto setexit; @@ -2485,7 +2490,11 @@ int sh_exec(register const Shnode_t *t, int flags) struct Ufunction *rp = np->nvalue.rp; slp = (struct slnod*)np->nvenv; sh_funstaks(slp->slchild,-1); - stakdelete(slp->slptr); + if(slp->slptr) + { + stakdelete(slp->slptr); + slp->slptr = NIL(Stak_t*); + } if(rp->sdict) { Namval_t *mp, *nq; @@ -2521,7 +2530,8 @@ int sh_exec(register const Shnode_t *t, int flags) struct comnod *ac = t->funct.functargs; slp = t->funct.functstak; sh_funstaks(slp->slchild,1); - staklink(slp->slptr); + if(slp->slptr) + staklink(slp->slptr); np->nvenv = (char*)slp; nv_funtree(np) = (int*)(t->funct.functtre); np->nvalue.rp->hoffset = t->funct.functloc; diff --git a/src/cmd/ksh93/tests/functions.sh b/src/cmd/ksh93/tests/functions.sh index 1cffed0b3..a9e295627 100755 --- a/src/cmd/ksh93/tests/functions.sh +++ b/src/cmd/ksh93/tests/functions.sh @@ -1400,5 +1400,100 @@ else unset exp fi +# ====== +# funcname.ksh crashed +# https://github.com/ksh93/ksh/issues/212 +cat >$tmp/funcname.ksh <<-'EOF' +# tweaked version of funname.ksh by Daniel Douglas +# https://gist.github.com/ormaaj/12874b68acd06ee98b59 +# Used by permission: "Consider all my gists MIT / do whatever." +# https://github.com/ksh93/ksh/issues/212#issuecomment-807915937 + +# run in subshell for additional regression testing +( + IFS='-' # element separator for "$*" etc., incl. "${FUNCNAME[*]}" + typeset -a FUNCNAME + + function FUNCNAME.get { + nameref self=${.sh.name} + if (( .sh.subscript < .sh.level )); then + trap "(( .sh.level -= .sh.subscript + 1 )); eval '(( .sh.level = ${.sh.level} ))' \; _=\${.sh.fun}" DEBUG + trap - DEBUG; + fi + + (( .sh.subscript < .sh.level - 2 )) && self[.sh.subscript + 1]= + } + + function f { + if (($1 < 10)); then + print -r -- "${FUNCNAME[*]}" + g $(($1 + 1)) + fi + print -r -- "${FUNCNAME[*]}" + } + + function g { + if (($1 < 10)); then + print -r -- "${FUNCNAME[*]}" + f $(($1 + 1)) + fi + print -r -- "${FUNCNAME[*]}" + } + + #typeset -ft FUNCNAME.get f g + f 0 +) +: do not optimize out the subshell +EOF +exp='f +g-f +f-g-f +g-f-g-f +f-g-f-g-f +g-f-g-f-g-f +f-g-f-g-f-g-f +g-f-g-f-g-f-g-f +f-g-f-g-f-g-f-g-f +g-f-g-f-g-f-g-f-g-f +f-g-f-g-f-g-f-g-f-g-f +g-f-g-f-g-f-g-f-g-f- +f-g-f-g-f-g-f-g-f-- +g-f-g-f-g-f-g-f--- +f-g-f-g-f-g-f---- +g-f-g-f-g-f----- +f-g-f-g-f------ +g-f-g-f------- +f-g-f-------- +g-f--------- +f----------' +got=$(set +x; { "$SHELL" funcname.ksh ;} 2>&1) +[[ $got == "$exp" ]] || err_exit 'funcname.ksh crash (direct run)' \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# dotting it slightly changes the output +exp='f- +g-f- +f-g-f- +g-f-g-f- +f-g-f-g-f- +g-f-g-f-g-f- +f-g-f-g-f-g-f- +g-f-g-f-g-f-g-f- +f-g-f-g-f-g-f-g-f- +g-f-g-f-g-f-g-f-g-f- +f-g-f-g-f-g-f-g-f-g-f- +g-f-g-f-g-f-g-f-g-f-- +f-g-f-g-f-g-f-g-f--- +g-f-g-f-g-f-g-f---- +f-g-f-g-f-g-f----- +g-f-g-f-g-f------ +f-g-f-g-f------- +g-f-g-f-------- +f-g-f--------- +g-f---------- +f-----------' +got=$(set +x; { "$SHELL" -c '. ./funcname.ksh' ;} 2>&1) +[[ $got == "$exp" ]] || err_exit 'funcname.ksh crash (dot)' \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125))