From 3aee10d781fd55970642cee9701e84fe67e50f88 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Mon, 28 Sep 2020 22:43:04 +0200
Subject: [PATCH] Fix off-by-one error, possible crash (re: 6193c6a3)

The ksh-20120801-trapcom.patch patch contains an off-by-one error,
which was also imported into 93u+m. When saving signals:

https://github.com/ksh93/ksh/blob/ceb77b136fb1f2cb0adf1e197c1b60d5b7c91d96/src/cmd/ksh93/sh/subshell.c#L572-L592
572	if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
573	{
574		++nsig;
575		savsig = malloc(nsig * sizeof(char*));
576		/*
577		 * the data is, usually, modified in code like:
578		 *	tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
579		 * so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
580		 */
581		for (isig = 0; isig < nsig; ++isig)
582		{
583			if(shp->st.trapcom[isig] == Empty)
584				savsig[isig] = Empty;
585			else if(shp->st.trapcom[isig])
586				savsig[isig] = strdup(shp->st.trapcom[isig]);
587			else
588				savsig[isig] = NULL;
589		}

On line 574, the number of signals 'nsig' is increased by one. That
increase is permanent, so the 'for' loop on line 581 tries to save
one signal state too many.

The increase was a holdout from the ksh93 code from before the
patch. After the patch, it is not required; it is fine to malloc as
many records as there are trapcom elements to save. So it should
simply be removed. xec.c has the same code to save trap states for
ksh functions, and the same applies.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Don't increase nsig.

src/cmd/ksh93/sh/xec.c: sh_funscope():
- Same.

src/cmd/ksh93/tests/signal.sh:
- Add test.
---
 src/cmd/ksh93/sh/subshell.c   |  1 -
 src/cmd/ksh93/sh/xec.c        |  1 -
 src/cmd/ksh93/tests/signal.sh | 14 ++++++++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index fdb020729036..cd98deb046e5 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -571,7 +571,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		shp->st.otrap = savst.trap;
 		if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
 		{
-			++nsig;
 			savsig = malloc(nsig * sizeof(char*));
 			/*
 			 * the data is, usually, modified in code like:
diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index e5b460abc12d..7abafe4b5bfa 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -3096,7 +3096,6 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
 	/* save trap table */
 	if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
 	{
-		++nsig;
 		savsig = malloc(nsig * sizeof(char*));
 		/*
 		 * the data is, usually, modified in code like:
diff --git a/src/cmd/ksh93/tests/signal.sh b/src/cmd/ksh93/tests/signal.sh
index 1bec29701530..c69e798abf29 100755
--- a/src/cmd/ksh93/tests/signal.sh
+++ b/src/cmd/ksh93/tests/signal.sh
@@ -480,5 +480,19 @@ then
 	kill -INFO $$ || err_exit '`kill` cannot send SIGINFO to processes when passed `-INFO`'
 fi
 
+# ======
+# Due to an off-by-one error, the last signal in 'kill -l' output wasn't treated properly and could crash.
+
+sig=$(kill -l | tail -n 1)
+exp="OK: $sig"
+
+got=$(export sig; "$SHELL" -c '
+	trap "print '\''OK: $sig'\''" "$sig"
+	(kill -s "$sig" "$$")
+	trap - "$sig"
+' 2>&1)
+((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "failed to handle SIG$sig from subshell" \
+	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))