From 232b7bff30082fadd599585297ff98a70aa701c2 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 8 Feb 2022 21:40:19 +0000 Subject: [PATCH] Fix multiple bugs in executing scripts without a #! path When executing a script without a hashbang path like #!/bin/ksh, ksh forks itself, longjmps back to sh_main(), and then (among other things) calling sh_reinit() which is the function that tries to reinitialise as much of the shell as it can. This is its way of ensuring the child script is run in ksh and not some other shell. However, this appraoch is incredibly buggy. Among other things, changes in built-in commands and custom type definitions survived the reinitialisation, "exporting" variables didn't work properly, and the hash table and ${.sh.stats} weren't reset. As a result, depending on what the invoking script did, the invoked script could easily fail or malfunction. It is not actually possible to reinitialise the shell correctly, because some of the shell state is in locally scoped static variables that cannot simply be reinitialised. There are probably huge memory leaks with this approach as well. At some point, all this is going to need a total redesign. Clearly, the only reliable way involves execve(2) and a start from scratch. For now though, this seems to fix the known bugs at least. I'm sure there are more to be discovered. This commit makes another change: instead of the -h/trackall option (which has been a no-op for decades), the posix option is now inherited by the child script. Since there is no hashbang path from which to decide whether the shell should run in POSIX mode nor not, the best guess is probably the invoking script's setting. src/cmd/ksh93/sh/init.c: sh_reinit(): - Keep the SH_INIT state on during the entire procedure. - Delete remaining non-exported, non-default variables. - Remove attributes from exported variables. In POSIX mode, remove all attributes; otherwise, only remove readonly. - Unset discipline function pointers for variables. - Delete all custom types. - Delete all functions and built-ins, then reinitialise the built-ins table from scatch. - Free the alias values before clearing the alias table. - Same with hash table entries (tracked aliases). - Reset statistics. - Inherit SH_POSIX instead of SH_TRACKALL. - Call user init function last, not somewhere in the middle. src/cmd/ksh93/sh/name.c: sh_envnolocal(): - Be sure to preserve the export attribute of kept variables. Resolves: https://github.com/ksh93/ksh/issues/350 --- NEWS | 5 ++ src/cmd/ksh93/sh.1 | 7 +- src/cmd/ksh93/sh/init.c | 165 ++++++++++++++++++++++++++--------- src/cmd/ksh93/sh/name.c | 4 +- src/cmd/ksh93/tests/types.sh | 49 +++++++++++ 5 files changed, 186 insertions(+), 44 deletions(-) diff --git a/NEWS b/NEWS index 8bb88dd38728..d219b2300680 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,11 @@ Any uppercase BUG_* names are modernish shell bug IDs. 2022-02-08: +- Multiple bugs were fixed in the method that ksh uses to execute a shell + script without a #! path. Among other things, custom types and other + changes in built-in commands will no longer affect the invoked script. + The state of the 'posix' option is now inherited by the script. + - To avoid an inconsistent state, the 'enum' and 'typeset -T' commands are no longer allowed to replace special built-in commands. For instance, 'enum trap=(a b c)' is now an error because 'trap' is a special built-in. diff --git a/src/cmd/ksh93/sh.1 b/src/cmd/ksh93/sh.1 index f408c88d483e..8c812a1fc3ed 100644 --- a/src/cmd/ksh93/sh.1 +++ b/src/cmd/ksh93/sh.1 @@ -7588,9 +7588,10 @@ shells. At the moment that the \fBposix\fR option is turned on, it also turns on \fBletoctal\fR and turns off \fB\-B\fR/\fBbraceexpand\fR; the reverse is done when \fBposix\fR is turned back off. (These options can still be controlled independently in between.) Furthermore, the \fBposix\fR option -is automatically turned on upon invocation if ksh is invoked as \fBsh\fR -or \fBrsh\fR. In that case, or if the option is turned on by -specifying \fB-o posix\fR on the invocation command line, the invoked shell +is automatically turned on upon invocation if the shell is invoked as \fBsh\fR +or \fBrsh\fR, or if \fB\-o posix\fR or \fB\-\-posix\fR is specified on the +shell invocation command line, or when executing scripts without a \fB#!\fR path +with this option active in the invoking shell. In that case, the invoked shell will not set the preset aliases even if interactive, and will not import type attributes for variables (such as integer or left/right justify) from the environment. diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 2ba1efb25dee..560c69159bbf 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -218,12 +218,14 @@ typedef struct _init_ #endif /* _hdr_locale */ } Init_t; -static Init_t *ip; static int lctype; -static int nbltins; +static int nvars; static char *env_init(void); static void env_import_attributes(char*); static Init_t *nv_init(void); +#if SHOPT_STATS +static void stat_init(void); +#endif static int shlvl; static int rand_shift; @@ -779,7 +781,7 @@ static void put_lastarg(Namval_t* np,const char *val,int flags,Namfun_t *fp) */ void sh_setmatch(const char *v, int vsize, int nmatch, regoff_t match[],int index) { - struct match *mp = &ip->SH_MATCH_init; + struct match *mp = &((Init_t*)sh.init_context)->SH_MATCH_init; Namval_t *np = (Namval_t*)(&(mp->node[0])); register int i,n,x; unsigned int savesub = sh.subshell; @@ -1484,44 +1486,27 @@ Shell_t *sh_init(register int argc,register char *argv[], Shinit_f userinit) } /* - * reinitialize before executing a script + * Reinitialize before executing a script without a #! path. + * This is done in a fork of the shell, so "exporting" is done by deleting all non-exported variables. */ int sh_reinit(char *argv[]) { Shopt_t opt; Namval_t *np,*npnext; Dt_t *dp; - for(np=dtfirst(sh.fun_tree);np;np=npnext) - { - if((dp=sh.fun_tree)->walk) - dp = dp->walk; - npnext = (Namval_t*)dtnext(sh.fun_tree,np); - if(np>= sh.bltin_cmds && np < &sh.bltin_cmds[nbltins]) - continue; - if(is_abuiltin(np) && nv_isattr(np,NV_EXPORT)) - continue; - if(*np->nvname=='/') - continue; - nv_delete(np,dp,NV_NOFREE); - } - dtclose(sh.alias_tree); - sh.alias_tree = dtopen(&_Nvdisc,Dtoset); + int nofree; + sh_onstate(SH_INIT); + sh.subshell = sh.realsubshell = sh.comsub = sh.curenv = sh.jobenv = sh.inuse_bits = sh.fn_depth = sh.dot_depth = 0; + sh.envlist = NIL(struct argnod*); sh.last_root = sh.var_tree; - sh.inuse_bits = 0; - if(sh.userinit) - (*sh.userinit)(&sh, 1); if(sh.heredocs) { sfclose(sh.heredocs); sh.heredocs = 0; } - /* remove locals */ - sh_onstate(SH_INIT); + /* Remove non-exported variables, first pass (see sh_envnolocal() in name.c) */ nv_scan(sh.var_tree,sh_envnolocal,NIL(void*),NV_EXPORT,0); nv_scan(sh.var_tree,sh_envnolocal,NIL(void*),NV_ARRAY,NV_ARRAY); - sh_offstate(SH_INIT); - memset(sh.st.trapcom,0,(sh.st.trapmax+1)*sizeof(char*)); - memset((void*)&opt,0,sizeof(opt)); #if SHOPT_NAMESPACE if(sh.namespace) { @@ -1532,8 +1517,106 @@ int sh_reinit(char *argv[]) sh.namespace = 0; } #endif /* SHOPT_NAMESPACE */ - if(sh_isoption(SH_TRACKALL)) - on_option(&opt,SH_TRACKALL); + /* Delete remaining non-exported, non-default variables; remove attributes from exported variables */ + for(np = dtfirst(sh.var_tree); np; np = npnext) + { + if((dp = sh.var_tree)->walk) + dp = dp->walk; + npnext = (Namval_t*)dtnext(sh.var_tree,np); + /* skip default variables (already handled by sh_envnolocal()) */ + if(np >= sh.bltin_nodes && np < &sh.bltin_nodes[nvars]) + continue; + if(nv_isattr(np,NV_EXPORT)) + { + if(sh_isoption(SH_POSIX)) + { + char *cp = NIL(char*); + /* do not export attributes */ + if(nv_isattr(np,NV_INTEGER)) /* any kind of numeric? */ + { + cp = sh_strdup(nv_getval(np)); /* save string value */ + _nv_unset(np,NV_RDONLY); /* free numeric value */ + } + nv_setattr(np,NV_EXPORT); /* turn off everything except export */ + if(cp) + np->nvalue.cp = cp; /* replace by string value */ + } + else + { + /* export all attributes except readonly */ + nv_offattr(np,NV_RDONLY); + } + /* unset discipline */ + if(np->nvfun && np->nvfun->disc) + np->nvfun->disc = NIL(const Namdisc_t*); + } + else + { + nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ + _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ + nv_delete(np,dp,nofree); + } + } + /* Delete types */ + for(np = dtfirst(sh.typedict); np; np = npnext) + { + if((dp = sh.typedict)->walk) + dp = dp->walk; + npnext = (Namval_t*)dtnext(sh.typedict,np); + nv_delete(np,dp,0); + } + /* Delete functions/built-ins. Note: fun_tree has a viewpath to bltin_tree */ + for(np = dtfirst(sh.fun_tree); np; np = npnext) + { + if((dp = sh.fun_tree)->walk) + dp = dp->walk; + npnext = (Namval_t*)dtnext(sh.fun_tree,np); + nv_delete(np, dp, NV_NOFREE); + } + while(dp = dtview(sh.fun_tree, NIL(Dt_t*))) + { + dtclose(sh.fun_tree); + sh.fun_tree = dp; + } + dtclear(sh.fun_base = sh.fun_tree); + /* Re-init built-ins as per nv_init() */ + free(sh.bltin_cmds); + sh.bltin_tree = sh_inittree((const struct shtable2*)shtab_builtins); + dtview(sh.fun_tree,sh.bltin_tree); + /* Delete aliases */ + for(np = dtfirst(sh.alias_tree); np; np = npnext) + { + if((dp = sh.alias_tree)->walk) + dp = dp->walk; + npnext = (Namval_t*)dtnext(sh.alias_tree,np); + nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ + _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ + nv_delete(np,dp,nofree); + } + /* Delete hash table entries */ + for(np = dtfirst(sh.track_tree); np; np = npnext) + { + if((dp = sh.track_tree)->walk) + dp = dp->walk; + npnext = (Namval_t*)dtnext(sh.track_tree,np); + nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ + _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ + nv_delete(np,dp,nofree); + } + while(dp = dtview(sh.track_tree, NIL(Dt_t*))) + { + dtclose(sh.track_tree); + sh.track_tree = dp; + } +#if SHOPT_STATS + /* Reset statistics */ + free(sh.stats); + stat_init(); +#endif + /* Reset shell options; inherit some */ + memset((void*)&opt,0,sizeof(opt)); + if(sh_isoption(SH_POSIX)) + on_option(&opt,SH_POSIX); #if SHOPT_ESH if(sh_isoption(SH_EMACS)) on_option(&opt,SH_EMACS); @@ -1552,12 +1635,12 @@ int sh_reinit(char *argv[]) sh.arglist = sh_argcreate(argv); if(sh.arglist) sh_argreset(sh.arglist,NIL(struct dolnod*)); - sh.envlist=0; - sh.curenv = 0; sh.shname = error_info.id = sh_strdup(sh.st.dolv[0]); sh_offstate(SH_FORKED); - sh.fn_depth = sh.dot_depth = 0; + /* Reset traps and signals */ + memset(sh.st.trapcom,0,(sh.st.trapmax+1)*sizeof(char*)); sh_sigreset(0); + /* increase SHLVL */ if(!(SHLVL->nvalue.ip)) { shlvl = 0; @@ -1576,8 +1659,12 @@ int sh_reinit(char *argv[]) sh.errtrap = 0; sh.end_fn = 0; /* update ${.sh.pid}, $$, $PPID */ + sh.ppid = sh.current_pid; sh.current_pid = sh.pid = getpid(); - sh.ppid = getppid(); + /* call user init function, if any */ + if(sh.userinit) + (*sh.userinit)(&sh, 1); + sh_offstate(SH_INIT); return(1); } @@ -1695,8 +1782,6 @@ static void stat_init(void) sp->hdr.nofree = 1; nv_setvtree(SH_STATS); } -#else -# define stat_init(x) #endif /* SHOPT_STATS */ /* @@ -1705,7 +1790,7 @@ static void stat_init(void) static Init_t *nv_init(void) { double d=0; - ip = sh_newof(0,Init_t,1,0); + Init_t *ip = sh_newof(0,Init_t,1,0); sh.nvfun.last = (char*)&sh; sh.nvfun.nofree = 1; sh.var_base = sh.var_tree = sh_inittree(shtab_variables); @@ -1841,13 +1926,13 @@ Dt_t *sh_inittree(const struct shtable2 *name_vals) for(tp=name_vals;*tp->sh_name;tp++) n++; np = (Namval_t*)sh_calloc(n,sizeof(Namval_t)); - if(!sh.bltin_nodes) + if(name_vals==shtab_variables) + { sh.bltin_nodes = np; + nvars = n; + } else if(name_vals==(const struct shtable2*)shtab_builtins) - { sh.bltin_cmds = np; - nbltins = n; - } base_treep = treep = dtopen(&_Nvdisc,Dtoset); treep->user = (void*)&sh; for(tp=name_vals;*tp->sh_name;tp++,np++) diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index 8f508a01d009..30c300e317f9 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -2383,8 +2383,8 @@ void sh_scope(struct argnod *envlist, int fun) void sh_envnolocal (register Namval_t *np, void *data) { + char *cp = 0, was_export = nv_isattr(np,NV_EXPORT)!=0; NOT_USED(data); - char *cp=0; if(np==VERSIONNOD && nv_isref(np)) return; if(np==L_ARGNOD) @@ -2419,6 +2419,8 @@ void sh_envnolocal (register Namval_t *np, void *data) nv_putval(np,cp,0); free((void*)cp); } + if(was_export) + nv_onattr(np,NV_EXPORT); } /* diff --git a/src/cmd/ksh93/tests/types.sh b/src/cmd/ksh93/tests/types.sh index b5e9331f696d..a9616044f2b9 100755 --- a/src/cmd/ksh93/tests/types.sh +++ b/src/cmd/ksh93/tests/types.sh @@ -672,5 +672,54 @@ exp=': trap: is a special shell builtin' [[ $got == *"$exp" ]] || err_exit "typeset -T overrides special builtin" \ "(expected match of *$(printf %q "$exp"); got $(printf %q "$got"))" +# ====== +# Bugs involving scripts without a #! path +# Hashbangless scripts are execeuted in a reinitialised fork of ksh, which is very bug-prone. +# https://github.com/ksh93/ksh/issues/350 +# Some of these fixed bugs don't involve types at all, but the tests need to go somewhere. +# Plus, invoking these from an environment with a bunch of types defined is an additional test. +: >|foo1 +: >|foo2 +chmod +x foo1 foo2 + +enum _foo1_t=(VERY BAD TYPE) +_foo1_t foo=BAD +normalvar=BAD2 +cat >|foo1 <<-'EOF' + # no hashbang path here + echo "normalvar=$normalvar" + echo "foo=$foo" + PATH=/dev/null + typeset -p .sh.type + _foo1_t --version +EOF +./foo1 >|foo1.out 2>&1 +got=$(/dev/null +then cat >|foo1 <<-'EOF' + PATH=/dev/null + basename --version + EOF + ./foo1 >|foo1.out 2>&1 + got=$(| foo1 +echo $'echo start2\ntypeset -p a\necho end2' >| foo2 +unset a +typeset -a a=(one two three) +export a +got=$(./foo1) +exp=$'start1\ntypeset -x a=one\nstart2\ntypeset -x a=one\nend2\nend1' +[[ $got == "$exp" ]] || err_exit 'exporting variable to #!-less script' \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125))