Fix use after free bug when using += (re: 75796a9c) (#466)

The previous fix for the += operator introduced a use-after-free
bug that could result in a variable pointing to random garbage:
   $ foo=bar
   $ foo+=_foo true
   $ typeset -p foo
   foo=V V
The use after free issue occurs because when nv_clone creates a
copy of $foo in the true command's invocation-local scope, it does
not duplicate the string $foo points to. As a result, the $foo
variable in the parent scope points to the same string as $foo in
the invocation-local scope, which causes the use after free bug
when cloned $foo variable is freed from memory.

src/cmd/ksh93/sh/nvdisc.c:
- To fix the use after free bug, allow nv_clone to duplicate the
  string with memdup or strdup when no flags are passed.

src/cmd/ksh93/tests/variables.sh:
- Add a regression test for using the += operator with regular
  commands.

src/cmd/ksh93/tests/leaks.sh:
- Add a regression test to ensure the bugfix doesn't introduce any
  memory leaks.

NEWS

@@ -3,7 +3,13 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
-2022-01-17:
+2022-02-18:
+
+- Fixed a regression introduced on 2021-04-11 that caused the += operator in
+  invocation-local assignments to crash the shell or modify variables outside
+  of the invocation-local scope.
+
+2022-02-17:
 
 - Fixed a crash, introduced on 2021-01-19, that occurred when using 'cd' in
   a subshell with the PWD variable unset.
@@ -11,7 +17,7 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Fixed a crash that could occur when or after entering the suspend character
   (Ctrl+Z) while the shell was blocked trying to write to a FIFO special file.
 
-2022-01-16:
+2022-02-16:
 
 - Backported minor additions to the 'read' built-in command from ksh 93v-:
   '-a' is now the same as '-A' and '-u p' is the same as '-p'. This is for

src/cmd/ksh93/include/version.h

@@ -21,7 +21,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-beta.2"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2022-02-17"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2022-02-18"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh/nvdisc.c

@@ -972,7 +972,7 @@ int nv_clone(Namval_t *np, Namval_t *mp, int flags)
 		mp->nvflag |= (np->nvflag&NV_MINIMAL);
 	if(mp->nvalue.cp==val && !nv_isattr(np,NV_INTEGER))
 	{
-		if(np->nvalue.cp && np->nvalue.cp!=Empty && (flags&NV_COMVAR) && !(flags&NV_MOVE))
+		if(np->nvalue.cp && np->nvalue.cp!=Empty && (!flags || ((flags&NV_COMVAR) && !(flags&NV_MOVE))))
 		{
 			if(size)
 				mp->nvalue.cp = (char*)sh_memdup(np->nvalue.cp,size);

src/cmd/ksh93/tests/leaks.sh

@@ -424,5 +424,22 @@ DO
 	ulimit --man 2>/dev/null
 DONE
 
+# ======
+# Possible memory leak when using the += operator on variables
+# in an invocation-local scope.
+bintrue=$(whence -p true)
+testfunc() { true; }
+alias testalias=testfunc
+TEST title='+= operator used before command'
+DO
+	baz=bar
+	baz+=baz :          # Special builtin
+	baz+=foo true       # Regular builtin
+	baz+=foo "$bintrue" # External command
+	baz+=foo testfunc   # Function
+	baz+=foo testalias  # Alias
+	unset baz
+DONE
+
 # ======
 exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/variables.sh

@@ -1356,5 +1356,14 @@ got=$("$SHELL" -c 'echo $SHLVL; "$SHELL" -c "echo \$SHLVL"; exec "$SHELL" -c "ec
 [[ $got == "$exp" ]] || err_exit "SHLVL not increased correctly" \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
+# ======
+# The += operator should not free variables outside of its
+# scope when used in an invocation-local assignment.
+exp='baz_foo
+baz'
+got=$("$SHELL" -c $'foo=baz; foo+=_foo "$SHELL" -c \'print $foo\'; print $foo')
+[[ $exp == "$got" ]] || err_exit "using the += operator for invocation-local assignments changes variables outside of the invocation-local scope" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))