Skip to content

Commit

Permalink
Fix unreliable behavior when special vars are readonly or unset
Browse files Browse the repository at this point in the history
src/cmd/ksh93/data/variables.c:
 - Running `unset .sh.lineno` creates a memory fault, so fix that
   by giving it the NV_NOFREE attribute. This crash was happening
   because ${.sh.lineno} is an integer that cannot be freed from
   memory with free(3).

src/cmd/ksh93/sh/init.c:
 - Tell _nv_unset to ignore NV_RDONLY when $RANDOM and $LINENO are
   restored from the subshell scope. This is required to fully
   restore the original state of these variables after a virtual
   subshell finishes.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/subshell.c:
 - Disabled an optimization for one instance of `sh_assignok` to
   fix `readonly` in virtual subshells. It fixes the following
   variables when `(readonly $varname); enum varname=` is run:

   $_
   ${.sh.name}
   ${.sh.subscript}
   ${.sh.level}

   The optimization in question prevents sh_assignok from saving the
   original state of these variables by making the sh_assignok call
   a no-op. Ksh needs the original state of a variable for it to be
   properly restored after a virtual subshell has run, otherwise ksh
   will simply carry over any new flags (being NV_RDONLY in this case)
   from the subshell into the main shell.

src/cmd/ksh93/tests/variables.sh:
 - Add regression tests from @McDutchie for setting special variables
   as readonly in virtual subshells and for unsetting special
   variables.

Fixes ksh93#4
  • Loading branch information
JohnoKing committed Jun 20, 2020
1 parent 9906535 commit ac9338f
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 6 deletions.
14 changes: 14 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,20 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2020-06-19:

- Fixed a bug that caused setting the following variables as readonly in
a virtual subshell to affect the environment outside of the subshell:
$_
${.sh.name}
${.sh.subscript}
${.sh.level}
$RANDOM
$LINENO

- Fixed a bug that caused `unset .sh.lineno` to memory fault because of an
invalid free.

2020-06-18:

- A two decade old bug that caused 'whence -a' to base the path of
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ static int setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp
if (tp->aflag && (tp->argnum>0 || (curflag!=newflag)))
{
if(shp->subshell)
sh_assignok(np,1);
sh_assignok(np,2);
if(troot!=shp->var_tree)
nv_setattr(np,newflag&~NV_ASSIGN);
else
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/data/variables.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const struct shtable2 shtab_variables[] =
".sh.fun", 0, (char*)0,
".sh.subshell", NV_INTEGER|NV_SHORT|NV_NOFREE, (char*)0,
".sh.level", 0, (char*)0,
".sh.lineno", NV_INTEGER, (char*)0,
".sh.lineno", NV_INTEGER|NV_NOFREE, (char*)0,
".sh.stats", 0, (char*)0,
".sh.math", 0, (char*)0,
".sh.pool", 0, (char*)0,
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
* David Korn <dgk@research.att.com> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-06-18"
#define SH_RELEASE "93u+m 2020-06-19"
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ static void put_rand(register Namval_t* np,const char *val,int flags,Namfun_t *f
fp = nv_stack(np, NIL(Namfun_t*));
if(fp && !fp->nofree)
free((void*)fp);
_nv_unset(np,0);
_nv_unset(np,NV_RDONLY);
return;
}
if(flags&NV_INTEGER)
Expand Down Expand Up @@ -696,7 +696,7 @@ static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
fp = nv_stack(np, NIL(Namfun_t*));
if(fp && !fp->nofree)
free((void*)fp);
_nv_unset(np,0);
_nv_unset(np,NV_RDONLY);
return;
}
if(flags&NV_INTEGER)
Expand Down
7 changes: 6 additions & 1 deletion src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ int nv_subsaved(register Namval_t *np)
* This routine will make a copy of the given node in the
* layer created by the most recent subshell_fork if the
* node hasn't already been copied
*
* add == 0: Move the node pointer from the parent shell to the current virtual subshell.
* add == 1: Create a copy of the node pointer in the current virtual subshell.
* add == 2: This will also copy the node pointer, but unlike 1 it will not skip copying the
* following variables: ${.sh.level}, $_, ${.sh.subscript} and ${.sh.name}.
*/
Namval_t *sh_assignok(register Namval_t *np,int add)
{
Expand All @@ -252,7 +257,7 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
if(sp->subshare)
return(np);
/* don't bother with this */
if(!sp->shpwd || np==SH_LEVELNOD || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)
if(!sp->shpwd || (add != 2 && (np==SH_LEVELNOD || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)))
return(np);
if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
{
Expand Down
113 changes: 113 additions & 0 deletions src/cmd/ksh93/tests/variables.sh
Original file line number Diff line number Diff line change
Expand Up @@ -795,5 +795,118 @@ expect=1
actual=$(env SHLVL="2#11+x[\$(env echo Exploited vuln CVE-2019-14868 >&2)0]" "$SHELL" -c 'echo $SHLVL' 2>&1)
[[ $actual == $expect ]] || err_exit "expression allowed on env var import (expected '$expect', got '$actual')"

# ======
# Check unset 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.
set -- \
"PS1" \
"PS2" \
"IFS" \
"PWD" \
"HOME" \
"MAIL" \
"REPLY" \
"SHELL" \
"EDITOR" \
"MAILCHECK" \
"RANDOM" \
"ENV" \
"HISTFILE" \
"HISTSIZE" \
"HISTEDIT" \
"HISTCMD" \
"FCEDIT" \
"CDPATH" \
"MAILPATH" \
"PS3" \
"OLDPWD" \
"VISUAL" \
"COLUMNS" \
"LINES" \
"PPID" \
"_" \
"TMOUT" \
"SECONDS" \
"LINENO" \
"OPTARG" \
"OPTIND" \
"PS4" \
"FPATH" \
"LANG" \
"LC_ALL" \
"LC_COLLATE" \
"LC_CTYPE" \
"LC_MESSAGES" \
"LC_NUMERIC" \
"FIGNORE" \
"KSH_VERSION" \
"JOBMAX" \
".sh.edchar" \
".sh.edcol" \
".sh.edtext" \
".sh.edmode" \
".sh.name" \
".sh.subscript" \
".sh.value" \
".sh.version" \
".sh.dollar" \
".sh.match" \
".sh.command" \
".sh.file" \
".sh.fun" \
".sh.lineno" \
".sh.subshell" \
".sh.level" \
".sh.stats" \
".sh.math" \
".sh.pool" \
"SHLVL" \
"CSWIDTH"

# ... unset
$SHELL -c '
errors=0
unset -v "$@" || let errors++
for var
do if [[ $var != "_" ]] && # only makes sense that $_ is immediately set again
{ [[ -v $var ]] || eval "[[ -n \${$var+s} ]]"; }
then echo " $0: special variable $var still set" >&2
let errors++
elif eval "[[ -n \${$var} ]]"
then echo " $0: special variable $var has value, though unset" >&2
let errors++
fi
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)"

# ... readonly in subshell
$SHELL -c '
errors=0
(
readonly "$@"
for var
do if (eval "$var=") 2>/dev/null
then echo " $0: special variable $var not made readonly in subshell" >&2
let errors++
fi
done
exit $errors
) || errors=$?
for var
do if ! (eval "$var=")
then echo " $0: special variable $var still readonly outside subshell" >&2
let errors++
fi
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)"

# ======
exit $((Errors<125?Errors:125))

0 comments on commit ac9338f

Please sign in to comment.