From e1c41bb2de3f0691e7875534495135b0617813f5 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 5 Sep 2020 19:31:28 +0200
Subject: [PATCH] Fix subshell leak for 3 special variables (re: 417883df,
 bd3e2a80)

Using a process of elimination I've identified ${.sh.level}
(SH_LEVELNOD) as the cause of the crash. This node apparently
cannot be copied or moved without destabilising the shell. It
contains the current depth of function calls and it cannot be
changed by assignment, so this is not actually a problem.
Meanwhile, this commit re-fixes it for the other three.

src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for L_ARGNOD,
  SH_SUBSCRNOD and SH_NAMENOD. 'add' now has 3 modes (0, 1, 2).
- The test for a ${ subshare; } was actually wrong. sp->subshare is
  a saved backup value. We must test shp->subshare. (re: a9de50bf)

src/cmd/ksh93/bltins/typeset.c:
- setall(): Update the mode 3 sh_assignok() call.

src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables except
  ${.sh.level}.
---
 src/cmd/ksh93/bltins/typeset.c   |  2 +-
 src/cmd/ksh93/sh/subshell.c      | 12 +++++-------
 src/cmd/ksh93/tests/variables.sh | 17 +++++++++++++++++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c
index 52b9c899e..d0a31da2a 100644
--- a/src/cmd/ksh93/bltins/typeset.c
+++ b/src/cmd/ksh93/bltins/typeset.c
@@ -800,7 +800,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,3);
+					sh_assignok(np,2);
 				if(troot!=shp->var_tree)
 					nv_setattr(np,newflag&~NV_ASSIGN);
 				else
diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index 87c7d65b2..504afb5ca 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -235,8 +235,6 @@ int nv_subsaved(register Namval_t *np)
  * add == 1:    Create a copy of the node pointer in the current virtual subshell.
  * add == 2:    This will create a copy of the node pointer like 1, but it will disable the
  *              optimization for ${.sh.level}.
- * add == 3:    This is like 1, but it will never skip the following variables:
- *              ${.sh.level}, $_, ${.sh.subscript} and ${.sh.name}.
  */
 Namval_t *sh_assignok(register Namval_t *np,int add)
 {
@@ -248,11 +246,11 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
 	Namval_t		*mpnext;
 	Namarr_t		*ap;
 	unsigned int		save;
-	/* don't bother to save if in a ${ subshare; } */
-	if(sp->subshare)
-		return(np);
-	/* don't bother with this */
-	if(!sp->shpwd || (add != 3 && ((add != 2 && np==SH_LEVELNOD) || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)))
+	/*
+	 * Don't save if told not to (see nv_restore()) or if we're in a ${ subshare; }.
+	 * Also, moving/copying ${.sh.level} (SH_LEVELNOD) may crash the shell.
+	 */
+	if(!sp->shpwd || shp->subshare || add<2 && np==SH_LEVELNOD)
 		return(np);
 	if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
 	{
diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh
index 0ffe75dd4..4fa7a1df1 100755
--- a/src/cmd/ksh93/tests/variables.sh
+++ b/src/cmd/ksh93/tests/variables.sh
@@ -1011,6 +1011,23 @@ $SHELL -c '
 e=$?
 ((e == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell (exit status $e)"
 
+# ... subshell leak test
+$SHELL -c '
+	errors=0
+	for var
+	do	if	[[ $var == .sh.level ]]
+		then	continue	# known to fail
+		fi
+		if	eval "($var=bug); [[ \${$var} == bug ]]" 2>/dev/null
+		then	echo "	$0: special variable $var leaks out of subshell" >&2
+			let errors++
+		fi
+	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)"
+
 # ======
 # ${.sh.pid} should be the forked subshell's PID
 (