From 18b3f4aa28182e2047957dd7edcf6f9d0ff1cc69 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sun, 27 Sep 2020 02:00:55 +0200
Subject: [PATCH] combining alarm and IFS caused segfault (rhbz#1176670)

The undocumented alarm builtin executes actions unsafely so that
'read' with an IFS assignment crashed when an alarm was triggered.

This applies an edited version of a Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-alarmifs.patch

Prior discussion:
https://bugzilla.redhat.com/1176670

src/cmd/ksh93/bltins/alarm.c:
- Add a TODO note based on dgk's 2014 email cited in the RH bug.
- When executing the trap function, save and restore the IFS table.

src/cmd/ksh93/sh/init.c: get_ifs():
- Remove now-unnecessary SHOPT_MULTIBYTE preprocessor directives as
  8477d2ce lets the compiler optimise out multibyte code if needed.
- Initialise the 0 position of the IFS table to S_EOF. This
  corresponds with the static state tables in data/lexstates.c.

src/cmd/ksh93/tests/builtins.sh:
- Crash test.
---
 src/cmd/ksh93/bltins/alarm.c    | 17 +++++++++++++++++
 src/cmd/ksh93/sh/init.c         | 10 ++--------
 src/cmd/ksh93/tests/builtins.sh | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/src/cmd/ksh93/bltins/alarm.c b/src/cmd/ksh93/bltins/alarm.c
index 50261c6a4..5d152cc16 100644
--- a/src/cmd/ksh93/bltins/alarm.c
+++ b/src/cmd/ksh93/bltins/alarm.c
@@ -26,6 +26,18 @@
  *
  */
 
+/*
+ * TODO: 2014 email from David Korn cited at <https://bugzilla.redhat.com/1176670>:
+ *
+ * > I never documented the alarm builtin because it is problematic.  The
+ * > problem is that traps can't safely be handled asynchronously.  What should
+ * > happen is that the trap is marked for execution (sh_trapnote) and run after
+ * > the current command completes.  The time trap should wake up the shell if
+ * > it is blocked and it should return and then handle the trap.
+ *
+ * When that is done, the save_ifstable workaround can probably be removed.
+ */
+
 #include	"defs.h"
 #include	<error.h>
 #include	<stak.h>
@@ -141,7 +153,12 @@ void	sh_timetraps(Shell_t *shp)
 			{
 				tp->flags &= ~L_FLAG;
 				if(tp->action)
+				{
+					char	save_ifstable[256];
+					memcpy(save_ifstable,shp->ifstable,sizeof(save_ifstable));
 					sh_fun(tp->action,tp->node,(char**)0);
+					memcpy(shp->ifstable,save_ifstable,sizeof(save_ifstable));
+				}
 				tp->flags &= ~L_FLAG;
 				if(!tp->flags)
 				{
diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
index 1a7f1f9b6..f643c978e 100644
--- a/src/cmd/ksh93/sh/init.c
+++ b/src/cmd/ksh93/sh/init.c
@@ -503,21 +503,14 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp)
 		memset(shp->ifstable,0,(1<<CHAR_BIT));
 		if(cp=value)
 		{
-#if SHOPT_MULTIBYTE
-			while(n=mbsize(cp),c= *(unsigned char*)cp)
-#else
-			while(c= *(unsigned char*)cp++)
-#endif /* SHOPT_MULTIBYTE */
+			while(n = mbsize(cp), c = *(unsigned char*)cp++)
 			{
-#if SHOPT_MULTIBYTE
-				cp++;
 				if(n>1)
 				{
 					cp += (n-1);
 					shp->ifstable[c] = S_MBYTE;
 					continue;
 				}
-#endif /* SHOPT_MULTIBYTE */
 				n = S_DELIM;
 				if(c== *cp)
 					cp++;
@@ -533,6 +526,7 @@ static char* get_ifs(register Namval_t* np, Namfun_t *fp)
 			shp->ifstable[' '] = shp->ifstable['\t'] = S_SPACE;
 			shp->ifstable['\n'] = S_NL;
 		}
+		shp->ifstable[0] = S_EOF;
 	}
 	return(value);
 }
diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh
index 2b3bf6bc5..cde60c6e3 100755
--- a/src/cmd/ksh93/tests/builtins.sh
+++ b/src/cmd/ksh93/tests/builtins.sh
@@ -1055,5 +1055,27 @@ do	case $bltin in
 			"(expected string containing $(printf %q "$expect"), got $(printf %q "$actual"))"
 done 3< <(builtin)
 
+# ======
+# The 'alarm' builtin could make 'read' crash due to IFS table corruption caused by unsafe asynchronous execution.
+# https://bugzilla.redhat.com/1176670
+if	(builtin alarm) 2>/dev/null
+then	got=$( { "$SHELL" -c '
+		builtin alarm
+		alarm -r alarm_handler +.001
+		i=0
+		function alarm_handler.alarm
+		{
+			let "(++i) > 100" && exit
+		}
+		while :; do
+			echo cargo,odds and ends,jetsam,junk,wreckage,castoffs,sea-drift
+		done | while IFS="," read arg1 arg2 arg3 arg4 junk; do
+			:
+		done
+	'; } 2>&1)
+	((!(e = $?))) || err_exit 'crash with alarm and IFS' \
+		"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
+fi
+
 # ======
 exit $((Errors<125?Errors:125))