From b40155fae8da22204be889fc8a52afcfed61abe2 Mon Sep 17 00:00:00 2001
From: Johnothan King <johnothanking@protonmail.com>
Date: Mon, 15 Nov 2021 16:46:47 -0800
Subject: [PATCH] Fix file descriptor leaks in the `hist` builtin (#336)

This commit fixes two file descriptor leaks in the hist built-in.
The bugfix for the first file descriptor leak was backported from
ksh2020. See:
https://github.com/att/ast/issues/872
https://github.com/att/ast/commit/73bd61b5

Reproducer:
  $ echo no
  $ hist -s no=yes

The second file descriptor leak occurs after a substitution error
in the hist built-in (this leak wasn't fixed in ksh2020).
Reproducer:
  $ echo no
  $ ls /proc/$$/fd
  $ hist -s no=yes
  $ hist -s no=yes
  $ ls /proc/$$/fd

src/cmd/ksh93/bltins/hist.c:
- Close leftover file descriptors when an error occurs and after
  'hist -s' runs a command.

src/cmd/ksh93/tests/builtins.sh:
- Add two regression tests for both of the file descriptor leaks.
---
 NEWS                            |  3 +++
 src/cmd/ksh93/bltins/hist.c     | 10 ++++++++--
 src/cmd/ksh93/tests/builtins.sh | 25 +++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 47201fd49..33e948d2b 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
   special floating point constants Inf and NaN so that $((inf)) and $((nan))
   refer to the variables by those names as the standard requires. (BUG_ARITHNAN)
 
+- Fixed two file descriptor leaks in the hist builtin that occurred when
+  the -s flag ran a command or encountered an error.
+
 2021-11-14:
 
 - Fixed: ksh crashed after unsetting .sh.match and then matching a pattern.
diff --git a/src/cmd/ksh93/bltins/hist.c b/src/cmd/ksh93/bltins/hist.c
index eab572848..2211dc7de 100644
--- a/src/cmd/ksh93/bltins/hist.c
+++ b/src/cmd/ksh93/bltins/hist.c
@@ -264,18 +264,23 @@ int	b_hist(int argc,char *argv[], Shbltin_t *context)
 	sh_onstate(SH_HISTORY);
 	sh_onstate(SH_VERBOSE);	/* echo lines as read */
 	if(replace)
+	{
 		hist_subst(error_info.id,fdo,replace);
+		sh_close(fdo);
+	}
 	else if(error_info.errors == 0)
 	{
 		char buff[IOBSIZE+1];
-		Sfio_t *iop = sfnew(NIL(Sfio_t*),buff,IOBSIZE,fdo,SF_READ);
+		Sfio_t *iop;
 		/* read in and run the command */
 		if(shp->hist_depth++ > HIST_RECURSE)
 		{
+			sh_close(fdo);
 			errormsg(SH_DICT,ERROR_exit(1),e_toodeep,"history");
 			UNREACHABLE();
 		}
-		sh_eval(iop,1);
+		iop = sfnew(NIL(Sfio_t*),buff,IOBSIZE,fdo,SF_READ);
+		sh_eval(iop,1); /* this will close fdo */
 		shp->hist_depth--;
 	}
 	else
@@ -313,6 +318,7 @@ static void hist_subst(const char *command,int fd,char *replace)
 	*newp++ =  0;
 	if((sp=sh_substitute(string,replace,newp))==0)
 	{
+		sh_close(fd);
 		errormsg(SH_DICT,ERROR_exit(1),e_subst,command);
 		UNREACHABLE();
 	}
diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh
index b1e4e068b..b3b51f390 100755
--- a/src/cmd/ksh93/tests/builtins.sh
+++ b/src/cmd/ksh93/tests/builtins.sh
@@ -1273,5 +1273,30 @@ got=$(	readonly v=foo
 [[ $got == "$exp" ]] || err_exit "prefixing special builtin with 'command' does not stop it from exiting the shell on error" \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
+# ======
+# https://github.com/att/ast/issues/872
+hist_leak=$tmp/hist_leak.sh
+print 'ulimit -n 15' > "$hist_leak"
+for ((i=0; i!=11; i++)) do
+	print 'true foo\nhist -s foo=bar 2> /dev/null' >> "$hist_leak"
+done
+print 'print OK' >> "$hist_leak"
+exp="OK"
+got="$($SHELL -i "$hist_leak" 2>&1)"
+[[ $exp == "$got" ]] || err_exit "file descriptor leak in hist builtin" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
+# File descriptor leak after hist builtin substitution error
+hist_error_leak=$tmp/hist_error_leak.sh
+print 'ulimit -n 15' > "$hist_error_leak"
+for ((i=0; i!=11; i++)) do
+	print 'hist -s no=yes 2> /dev/null' >> "$hist_error_leak"
+done
+print 'print OK' >> "$hist_error_leak"
+exp="OK"
+got="$($SHELL -i "$hist_error_leak" 2>&1)"
+[[ $exp == "$got" ]] || err_exit "file descriptor leak after substitution error in hist builtin" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))