From 120aec25ba1a38a0c4672d9102a342571beb5f0a Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 1 Jul 2020 10:14:10 -0700 Subject: [PATCH] Fix a crash when 'read -u' is given an invalid fd (#53) File descriptors that are too far out of range will cause the read builtin to crash. The following example will generate two crashes: $ ksh -c 'read -u 2000000' || ksh -c 'read -u-2000000' The fix is to error out when the given file descriptor is out of range. This bugfix is from Tomas Klacko, although it was modified to use 'sh_iovalidfd' and reject numbers greater than 'INT_MAX': https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01912.html The question about 'shp->fdstatus[-1]' only applies to ksh93v- (ksh93u+ doesn't have any references to 'shp->fdstatus[-1]'). src/cmd/ksh93/bltins/read.c: - File descriptors that are out of range should be rejected with an error message (like invalid file descriptors that are in range). The seemingly redundant check for negative numbers is there because out of range negative numbers also cause memory faults despite the later 'fd<0' check. src/cmd/ksh93/tests/io.sh: - Add three tests for attempting 'read -u' on various invalid file descriptor numbers. --- NEWS | 5 +++++ src/cmd/ksh93/bltins/read.c | 2 ++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/tests/io.sh | 13 +++++++++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 54bf61d7b..9ab5949a6 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2020-06-30: + +- 'read -u' will no longer crash with a memory fault when given an out of + range or negative file descriptor. + 2020-06-28: - Variables created with 'typeset -RF' no longer cause a memory fault diff --git a/src/cmd/ksh93/bltins/read.c b/src/cmd/ksh93/bltins/read.c index 358908b52..a94657a07 100644 --- a/src/cmd/ksh93/bltins/read.c +++ b/src/cmd/ksh93/bltins/read.c @@ -128,6 +128,8 @@ int b_read(int argc,char *argv[], Shbltin_t *context) break; case 'u': fd = (int)opt_info.num; + if(opt_info.num<0 || opt_info.num>INT_MAX || (fd>=shp->gd->lim.open_max && !sh_iovalidfd(shp,fd))) + errormsg(SH_DICT,ERROR_exit(1),e_file,opt_info.arg); /* reject invalid file descriptors */ if(sh_inuse(shp,fd)) fd = -1; break; diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 5ef73135a..a412613f1 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -17,4 +17,4 @@ * David Korn * * * ***********************************************************************/ -#define SH_RELEASE "93u+m 2020-06-28" +#define SH_RELEASE "93u+m 2020-06-30" diff --git a/src/cmd/ksh93/tests/io.sh b/src/cmd/ksh93/tests/io.sh index 963d645eb..51eba6ce0 100755 --- a/src/cmd/ksh93/tests/io.sh +++ b/src/cmd/ksh93/tests/io.sh @@ -557,5 +557,18 @@ result=$("$SHELL" -ic 'echo >(true) >/dev/null' 2>&1) [[ -z $result ]] || err_exit 'interactive shells print a PID during process substitution' \ "(expected '', got $(printf %q "$result"))" +# ====== +# Out of range file descriptors shouldn't cause 'read -u' to segfault +"$SHELL" -c 'read -u2000000' 2> /dev/null +[[ $? == 1 ]] || err_exit "Large file descriptors cause 'read -u' to crash" + +# Separately test numbers outside of the 32-bit limit as well +"$SHELL" -c 'read -u2000000000000' 2> /dev/null +[[ $? == 1 ]] || err_exit "File descriptors larger than the 32-bit limit cause 'read -u' to crash" + +# Negative numbers shouldn't segfault either +"$SHELL" -c 'read -u-2000000' 2> /dev/null +[[ $? == 1 ]] || err_exit "Negative file descriptors cause 'read -u' to crash" + # ====== exit $((Errors<125?Errors:125))