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.
ksh93 · Sep 24, 2020 · 3654ee7 · 3654ee7
1 parent 843b546
commit 3654ee7
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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