From b3d37b00b0d46d434c90b4e5a3673fc3f5d5a197 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 5 Sep 2020 13:57:39 +0200
Subject: [PATCH] Fix subshell leak for 4 special variables (re: bd3e2a80)

The following special variables leaked out of a subshell:
$_, ${.sh.name}, ${.sh.level}, ${.sh.subscript}.
This was due to a faulty optimisation in sh_assignok().
bd3e2a80 fixed that in part, this fixes the rest.

src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for these four
  special variables. The 'add' param reverts to a simple boolean.
- 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(), unall(): Update sh_assignok() calls.

src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables.

Closes: #122
---
 src/cmd/ksh93/bltins/typeset.c   |  6 +++---
 src/cmd/ksh93/sh/subshell.c      | 11 ++---------
 src/cmd/ksh93/tests/variables.sh | 14 ++++++++++++++
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c
index 52b9c899e..a1591883e 100644
--- a/src/cmd/ksh93/bltins/typeset.c
+++ b/src/cmd/ksh93/bltins/typeset.c
@@ -688,7 +688,7 @@ static int     setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp
 					 * Variables with internal trap/discipline functions (LC_*, LINENO, etc.) need to be
 					 * cloned, as moving them will remove the discipline function.
 					 */
-					np=sh_assignok(np,2);
+					np=sh_assignok(np,1);
 				}
 				else
 					np=sh_assignok(np,0);
@@ -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,1);
 				if(troot!=shp->var_tree)
 					nv_setattr(np,newflag&~NV_ASSIGN);
 				else
@@ -1285,7 +1285,7 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
 						 * Variables with internal trap/discipline functions (LC_*, LINENO, etc.) need to be
 						 * cloned, as moving them will remove the discipline function.
 						 */
-						np=sh_assignok(np,2);
+						np=sh_assignok(np,1);
 					}
 					else
 						np=sh_assignok(np,0);
diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index 87c7d65b2..5ecbc94ce 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -233,10 +233,6 @@ int nv_subsaved(register Namval_t *np)
  *
  * 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 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 +244,8 @@ 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; } */
+	if(!sp->shpwd || shp->subshare)
 		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 0eb918db9..3604c1267 100755
--- a/src/cmd/ksh93/tests/variables.sh
+++ b/src/cmd/ksh93/tests/variables.sh
@@ -1011,6 +1011,20 @@ $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	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
 (