Fix an infinite loop related to $_ if ksh is /bin/sh (#90)

JohnoKing · web-flow · commit 8c16f38a8886 · 2020-07-24T01:20:26.000+01:00
The following explanation is mostly taken from Tomas Klacko's report on the old mailing list (which also contains a C program reproducer) [*]: 1. When ksh starts a binary, it sets its environment variable "_" to "*number*/path/to/binary". Where "number" is the pid of the ksh process. 2. The binary forks and the child executes a suid root shell script which begins with #!/bin/sh. For this bug to occur, ksh must be /bin/sh. 3. The ksh process interpreting the suid shell script leaves the "_" variable as not set (nv_getval(L_ARGNOD) returns NULL) because the "number" from step 1 is not the pid of its parent process. 4-5. Because "_" is not set and the script is suid root, an infinite loop occurs because when the SHELL environment variable contains "/bin/sh" pathshell() returns "/bin/sh". This becomes an infinite loop of /bin/sh /dev/fd/3 executing /bin/sh /dev/fd/3. src/cmd/ksh93/sh/init.c: get_lastarg(): - Disable the check for if the "number" refers to the process id of the parent process. src/cmd/ksh93/sh/main.c: sh_main(): - Prevent an infinite loop when '$_' is not passed in from the environment. Solaris applies this bugfix to their version of ksh: https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/190-17432413.patch [*]: https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01680.html
diff --git a/NEWS b/NEWS
@@ -5,6 +5,8 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 
 2020-07-23:
 
+- Fixed an infinite loop that could occur when ksh is the system's /bin/sh.
+
 - A command substitution that is run on the same line as a here-document
   will no longer cause a syntax error.
 
diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
@@ -710,7 +710,7 @@ static char* get_lastarg(Namval_t* np, Namfun_t *fp)
 	char	*cp;
 	int	pid;
         if(sh_isstate(SH_INIT) && (cp=shp->lastarg) && *cp=='*' && (pid=strtol(cp+1,&cp,10)) && *cp=='*')
-		nv_putval(np,(pid==shp->gd->ppid?cp+1:0),0);
+		nv_putval(np,cp+1,0);
 	return(shp->lastarg);
 }
 
diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c
@@ -273,7 +273,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
 					 * try to undo effect of solaris 2.5+
 					 * change for argv for setuid scripts
 					 */
-					if(((type = sh_type(cp = av[0])) & SH_TYPE_SH) && (!(name = nv_getval(L_ARGNOD)) || !((type = sh_type(cp = name)) & SH_TYPE_SH)))
+					if(((type = sh_type(cp = av[0])) & SH_TYPE_SH) && (name = nv_getval(L_ARGNOD)) && (!((type = sh_type(cp = name)) & SH_TYPE_SH)))
 					{
 						av[0] = (type & SH_TYPE_LOGIN) ? cp : path_basename(cp);
 						/*  exec to change $0 for ps */

Original file line number	Diff line number	Diff line change
`@@ -710,7 +710,7 @@ static char* get_lastarg(Namval_t* np, Namfun_t *fp)`
`710`	`710`	`char *cp;`
`711`	`711`	`int pid;`
`712`	`712`	`if(sh_isstate(SH_INIT) && (cp=shp->lastarg) && cp=='' && (pid=strtol(cp+1,&cp,10)) && cp=='')`
`713`		`- nv_putval(np,(pid==shp->gd->ppid?cp+1:0),0);`
	`713`	`+ nv_putval(np,cp+1,0);`
`714`	`714`	`return(shp->lastarg);`
`715`	`715`	`}`
`716`	`716`
Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)`
`273`	`273`	`* try to undo effect of solaris 2.5+`
`274`	`274`	`* change for argv for setuid scripts`
`275`	`275`	`*/`
`276`		`- if(((type = sh_type(cp = av[0])) & SH_TYPE_SH) && (!(name = nv_getval(L_ARGNOD)) \|\| !((type = sh_type(cp = name)) & SH_TYPE_SH)))`
	`276`	`+ if(((type = sh_type(cp = av[0])) & SH_TYPE_SH) && (name = nv_getval(L_ARGNOD)) && (!((type = sh_type(cp = name)) & SH_TYPE_SH)))`
`277`	`277`	`{`
`278`	`278`	`av[0] = (type & SH_TYPE_LOGIN) ? cp : path_basename(cp);`
`279`	`279`	`/* exec to change $0 for ps */`