From 3654ee73c03fb622dc878d6d2adb54dff13e0624 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Wed, 23 Sep 2020 19:15:45 +0200 Subject: [PATCH] Fix typeset -l/-u crash on special vars (rhbz#1083713) When using typeset -l or -u on a variable that cannot be changed when the shell is in restricted mode, ksh crashed. This fixed is inspired by this Red Hat fix, which is incomplete: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch The crash was caused by the nv_shell() function. It walks though a discipline function tree to get the pointer to the interpreter associated with it. Evidently, the problem is that some pointer in that walk is not set correctly for all special variables. Thing is, ksh only has one shell language interpreter, and only one global data structure (called 'sh') to keep its main state[*]. Yet, the code is full of 'shp' pointers to that structure. Most (not all) functions pass that pointer around to each other, accessing that struct indirectly, ostensibly to account for the non-existent possibility that there might be more than one interpreter state. The "why" of that is an interesting cause for speculation that I may get to sometime. For now, it is enough to know that, in the code as it is, it matters not one iota what pointer to the shell interpreter state is used; they all point to the same thing (unless it's broken, as in this bug). So, rather than fixing nv_shell() and/or associated pointer assignments, this commit simply removes it, and replaces it with calls to sh_getinterp(), which always returns a pointer to sh (see init.c, where that function is defined as literally 'return &sh'). [*] Defined in shell.h, with the _SH_PRIVATE part in defs.h src/cmd/ksh93/include/defs.h, src/cmd/ksh93/sh/name.c: - Remove nv_shell(). src/cmd/ksh93/sh/init.c: - In all the discipline functions for special variables, initialise shp using sh_getinterp() instead of nv_shell(). src/cmd/ksh93/tests/variables.sh: - Add regression test for typeset -l/-u on all special variables. --- NEWS | 3 ++ src/cmd/ksh93/include/defs.h | 1 - src/cmd/ksh93/sh/init.c | 28 ++++++++--------- src/cmd/ksh93/sh/name.c | 11 ------- src/cmd/ksh93/tests/variables.sh | 54 ++++++++++++++++++++++++++------ 5 files changed, 62 insertions(+), 35 deletions(-) diff --git a/NEWS b/NEWS index f5b7d122f0ee..4769e0f0ef80 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. - Fixed a crash that could occur when running a pipeline containing backtick-style command substitutions with job control enabled. +- Fixed a crash that occurred when using 'typeset -u' or 'typeset -l' on a + special variable such as PATH, ENV or SHELL. + 2020-09-21: - A bug was fixed that caused command substitutions embedded in here-documents diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index fe833506cdb9..7fc5160ff872 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -359,7 +359,6 @@ struct shared #define SH_FUNEVAL 0x10000 /* for sh_eval for function load */ extern struct shared *shgd; -extern Shell_t *nv_shell(Namval_t*); extern void sh_applyopts(Shell_t*,Shopt_t); extern char **sh_argbuild(Shell_t*,int*,const struct comnod*,int); extern struct dolnod *sh_argfree(Shell_t *, struct dolnod*,int); diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 90bf2681decc..392860baffc2 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -227,7 +227,7 @@ static void put_ed(register Namval_t* np,const char *val,int flags,Namfun_t *fp) { register const char *cp, *name=nv_name(np); register int newopt=0; - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); if(*name=='E' && nv_getval(sh_scoped(shp,VISINOD))) goto done; if(!(cp=val) && (*name=='E' || !(cp=nv_getval(sh_scoped(shp,EDITNOD))))) @@ -254,7 +254,7 @@ static void put_ed(register Namval_t* np,const char *val,int flags,Namfun_t *fp) /* Trap for HISTFILE and HISTSIZE variables */ static void put_history(register Namval_t* np,const char *val,int flags,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); void *histopen = shp->gd->hist_ptr; char *cp; if(val && histopen) @@ -278,7 +278,7 @@ static void put_history(register Namval_t* np,const char *val,int flags,Namfun_t /* Trap for OPTINDEX */ static void put_optindex(Namval_t* np,const char *val,int flags,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); shp->st.opterror = shp->st.optchar = 0; nv_putv(np, val, flags, fp); if(!val) @@ -303,7 +303,7 @@ static Namfun_t *clone_optindex(Namval_t* np, Namval_t *mp, int flags, Namfun_t /* Trap for restricted variables FPATH, PATH, SHELL, ENV */ static void put_restricted(register Namval_t* np,const char *val,int flags,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); int path_scoped = 0, fpath_scoped=0; Pathcomp_t *pp; char *name = nv_name(np); @@ -345,7 +345,7 @@ static void put_restricted(register Namval_t* np,const char *val,int flags,Namfu static void put_cdpath(register Namval_t* np,const char *val,int flags,Namfun_t *fp) { Pathcomp_t *pp; - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); nv_putv(np, val, flags, fp); if(!shp->cdpathlist) return; @@ -369,7 +369,7 @@ static void put_cdpath(register Namval_t* np,const char *val,int flags,Namfun_t /* Trap for LC_ALL, LC_CTYPE, LC_MESSAGES, LC_COLLATE and LANG */ static void put_lang(Namval_t* np,const char *val,int flags,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); int type; char *name = nv_name(np); if(name==(LCALLNOD)->nvname) @@ -494,7 +494,7 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp) register struct ifs *ip = (struct ifs*)fp; register char *cp, *value; register int c,n; - register Shell_t *shp = nv_shell(np); + register Shell_t *shp = sh_getinterp(); value = nv_getv(np,fp); if(np!=ip->ifsnp) { @@ -573,7 +573,7 @@ static void put_seconds(register Namval_t* np,const char *val,int flags,Namfun_t static char* get_seconds(register Namval_t* np, Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); register int places = nv_size(np); struct tms tp; double d, offset = (np->nvalue.dp?*np->nvalue.dp:0); @@ -657,7 +657,7 @@ static Sfdouble_t nget_lineno(Namval_t* np, Namfun_t *fp) static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp) { register long n; - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); if(!val) { fp = nv_stack(np, NIL(Namfun_t*)); @@ -681,7 +681,7 @@ static char* get_lineno(register Namval_t* np, Namfun_t *fp) static char* get_lastarg(Namval_t* np, Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); char *cp; int pid; if(sh_isstate(SH_INIT) && (cp=shp->lastarg) && *cp=='*' && (pid=strtol(cp+1,&cp,10)) && *cp=='*') @@ -691,7 +691,7 @@ static char* get_lastarg(Namval_t* np, Namfun_t *fp) static void put_lastarg(Namval_t* np,const char *val,int flags,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); if(flags&NV_INTEGER) { sfprintf(shp->strbuf,"%.*g",12,*((double*)val)); @@ -934,7 +934,7 @@ static void math_init(Shell_t *shp) static Namval_t *create_math(Namval_t *np,const char *name,int flag,Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); if(!name) return(SH_MATHNOD); if(name[0]!='a' || name[1]!='r' || name[2]!='g' || name[4] || !isdigit(name[3]) || (name[3]=='0' || (name[3]-'0')>MAX_MATH_ARGS)) @@ -945,7 +945,7 @@ static Namval_t *create_math(Namval_t *np,const char *name,int flag,Namfun_t *fp static char* get_math(register Namval_t* np, Namfun_t *fp) { - Shell_t *shp = nv_shell(np); + Shell_t *shp = sh_getinterp(); Namval_t *mp,fake; char *val; int first=0; @@ -966,7 +966,7 @@ static char* get_math(register Namval_t* np, Namfun_t *fp) static char *setdisc_any(Namval_t *np, const char *event, Namval_t *action, Namfun_t *fp) { - Shell_t *shp=nv_shell(np); + Shell_t *shp=sh_getinterp(); Namval_t *mp,fake; char *name; int getname=0, off=staktell(); diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index 2a0c2163da43..59ff09d8cb54 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -3624,17 +3624,6 @@ int nv_setsize(register Namval_t *np, int size) return(oldsize); } -Shell_t *nv_shell(Namval_t *np) -{ - Namfun_t *fp; - for(fp=np->nvfun;fp;fp=fp->next) - { - if(!fp->disc) - return((Shell_t*)fp->last); - } - return(0); -} - #undef nv_unset void nv_unset(register Namval_t *np) diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index d378358bb45b..bf623c3fd322 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -868,7 +868,7 @@ actual=$(env SHLVL="2#11+x[\$(env echo Exploited vuln CVE-2019-14868 >&2)0]" "$S [[ $actual == $expect ]] || err_exit "expression allowed on env var import (expected '$expect', got '$actual')" # ====== -# Check unset and cleanup/restore behavior of special variables. +# Check unset, attribute and cleanup/restore behavior of special variables. # Keep the list in sync (minus ".sh") with shtab_variables[] in src/cmd/ksh93/data/variables.c # Note: as long as changing $PATH forks a virtual subshell, "PATH" should also be excluded below. @@ -954,8 +954,8 @@ $SHELL -c ' done exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0 ' unset_test "$@" -e=$? -((e == 1)) || err_exit "Failure in unsetting one or more special variables (exit status $e)" +(((e = $?) == 1)) || err_exit "Failure in unsetting one or more special variables" \ + "(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" # ... unset in virtual subshell inside of nested function $SHELL -c ' @@ -984,8 +984,8 @@ $SHELL -c ' fun1 "$@" exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0 ' unset_subsh_fun_test "$@" -e=$? -((e == 1)) || err_exit "Unset of special variable(s) in a virtual subshell within a nested function fails (exit status $e)" +(((e = $?) == 1)) || err_exit "Unset of special variable(s) in a virtual subshell within a nested function fails" \ + "(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" # ... readonly in subshell $SHELL -c ' @@ -1008,8 +1008,8 @@ $SHELL -c ' done exit $((errors + 1)) # a possible erroneous asynchronous fork would cause exit status 0 ' readonly_test "$@" -e=$? -((e == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell (exit status $e)" +(((e = $?) == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell" \ + "(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" # ... subshell leak test $SHELL -c ' @@ -1025,8 +1025,44 @@ $SHELL -c ' done exit $((errors + 1)) ' subshell_leak_test "$@" -e=$? -((e == 1)) || err_exit "One or more special variables leak out of a subshell (exit status $e)" +(((e = $?) == 1)) || err_exit "One or more special variables leak out of a subshell" \ + "(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" + +# ... upper/lowercase test +$SHELL -c ' + typeset -u upper + typeset -l lower + errors=0 + PS1=/dev/null/test_my_case_too + PS2=$PS1 PS3=$PS1 PS4=$PS1 OPTARG=$PS1 IFS=$PS1 FPATH=$PS1 FIGNORE=$PS1 CSWIDTH=$PS1 + for var + do case $var in + RANDOM | HISTCMD | _ | SECONDS | LINENO | JOBMAX | .sh.stats) + # these are expected to fail below as their values change; just test against crashing + typeset -u "$var" + typeset -l "$var" + continue ;; + esac + nameref val=$var + upper=$val + lower=$val + typeset -u "$var" + if [[ $val != "$upper" ]] + then echo " $0: typeset -u does not work on special variable $var" \ + "(expected $(printf %q "$upper"), got $(printf %q "$val"))" >&2 + let errors++ + fi + typeset -l "$var" + if [[ $val != "$lower" ]] + then echo " $0: typeset -l does not work on special variable $var" \ + "(expected $(printf %q "$lower"), got $(printf %q "$val"))" >&2 + let errors++ + fi + done + exit $((errors + 1)) +' changecase_test "$@" PATH # do include PATH here as well +(((e = $?) == 1)) || err_exit "typeset -l/-u doesn't work on special variables" \ + "(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" # ====== # ${.sh.pid} should be the forked subshell's PID