From 11177d448dadc7f8300e1db60c4ea5bdd61f13e0 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 17 Feb 2022 16:19:17 +0000 Subject: [PATCH] Fix crash on cd in subshell with PWD unset (re: 5ee290c) Reproducer: $ ksh -c 'unset PWD; (cd /); :' Memory fault The shell crashes because b_cd() is testing the value of the PWD variable without checking if there is one. src/cmd/ksh93/sh/path.c: path_pwd(): - Never return an unfreeable pointer to e_dot; always return a freeable pointer. This fixes another corner-case crashing bug. - Make sure the PWD variable gets assigned a value if it doesn't have one, even if it's the "." fallback. However, if the PWD is inaccessible but we did inherit a $PWD value that starts with a /, then use the existing $PWD value as this will help the shell fail gracefully. src/cmd/ksh93/bltins/cd_pwd.c: - b_cd(): When checking if the PWD is valid, use the sh.pwd copy instead of the PWD variable. This fixes the crash above. - b_cd(): Since path_pwd() now always returns a freeable value, free sh.pwd unconditionally before setting the new value. - b_pwd(): Not only check that path_pwd() returns a value starting with a slash, but also verify it with test_inode() and error out if it's wrong. This makes the 'pwd' command useful for checking that the PWD is currently accessible. src/cmd/ksh93/data/msg.c: - Change e_pwd error message for accuracy and clarity. --- NEWS | 5 +++++ src/cmd/ksh93/bltins/cd_pwd.c | 13 ++++--------- src/cmd/ksh93/data/msg.c | 2 +- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/path.c | 31 +++++++++++++++++++++---------- src/cmd/ksh93/tests/path.sh | 14 ++++++++++++++ 6 files changed, 46 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 29b8c7174..d0fbee9e3 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0 Any uppercase BUG_* names are modernish shell bug IDs. +2022-01-17: + +- Fixed a crash, introduced on 2021-01-19, that occurred when using 'cd' in + a subshell with the PWD variable unset. + 2022-01-16: - Backported minor additions to the 'read' built-in command from ksh 93v-: diff --git a/src/cmd/ksh93/bltins/cd_pwd.c b/src/cmd/ksh93/bltins/cd_pwd.c index a40af756b..6818d7d95 100644 --- a/src/cmd/ksh93/bltins/cd_pwd.c +++ b/src/cmd/ksh93/bltins/cd_pwd.c @@ -100,8 +100,6 @@ int b_cd(int argc, char *argv[],Shbltin_t *context) oldpwd = path_pwd(); opwdnod = sh_scoped(OLDPWDNOD); pwdnod = sh_scoped(PWDNOD); - if(oldpwd == e_dot && pwdnod->nvalue.cp) - oldpwd = (char*)pwdnod->nvalue.cp; /* if path_pwd() failed to get the pwd, use $PWD */ if(sh.subshell) { /* clone $OLDPWD and $PWD into the subshell's scope */ @@ -126,7 +124,7 @@ int b_cd(int argc, char *argv[],Shbltin_t *context) if(sh.subshell && !sh.subshare) { #if _lib_fchdir - if(!test_inode(nv_getval(pwdnod),e_dot)) + if(!test_inode(sh.pwd,e_dot)) #endif sh_subfork(); } @@ -228,6 +226,7 @@ success: if(*dp && (*dp!='.'||dp[1]) && strchr(dir,'/')) sfputr(sfstdout,dir,'\n'); nv_putval(opwdnod,oldpwd,NV_RDONLY); + free((void*)sh.pwd); if(*dir == '/') { size_t len = strlen(dir); @@ -236,16 +235,12 @@ success: dir[len] = 0; nv_putval(pwdnod,dir,NV_RDONLY); nv_onattr(pwdnod,NV_EXPORT); - if(sh.pwd) - free((void*)sh.pwd); - sh.pwd = sh_strdup(pwdnod->nvalue.cp); + sh.pwd = sh_strdup(dir); } else { /* pathcanon() failed to canonicalize the directory, which happens when 'cd' is invoked from a nonexistent PWD with a relative path as the argument. Reinitialize $PWD as it will be wrong. */ - if(sh.pwd) - free((void*)sh.pwd); sh.pwd = NIL(const char*); path_pwd(); if(*sh.pwd != '/') @@ -291,7 +286,7 @@ int b_pwd(int argc, char *argv[],Shbltin_t *context) errormsg(SH_DICT,ERROR_usage(2),"%s",optusage((char*)0)); UNREACHABLE(); } - if(*(cp = path_pwd()) != '/') + if(*(cp = path_pwd()) != '/' || !test_inode(cp,e_dot)) { errormsg(SH_DICT,ERROR_system(1), e_pwd); UNREACHABLE(); diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c index 2be92a457..c19d0fcb0 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -78,7 +78,7 @@ const char e_badpattern[] = "%s: invalid shell pattern"; const char e_noread[] = "%s: pattern seek requires read access"; const char e_logout[] = "Use 'exit' to terminate this shell"; const char e_exec[] = "%s: cannot execute"; -const char e_pwd[] = "cannot access parent directories"; +const char e_pwd[] = "cannot determine present working directory"; const char e_found[] = "%s: not found"; #ifdef ENAMETOOLONG const char e_toolong[] = "%s: file name too long"; diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 4a7bb3a6e..70c073027 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -21,7 +21,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2022-02-16" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-02-17" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index fdf685595..f106ffa20 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -186,7 +186,11 @@ char *path_pwd(void) Namval_t *pwdnod; /* Don't bother if PWD already set */ if(sh.pwd) - return((char*)sh.pwd); + { + if(*sh.pwd=='/') + return((char*)sh.pwd); + free((void*)sh.pwd); + } /* First see if PWD variable is correct */ pwdnod = sh_scoped(PWDNOD); cp = nv_getval(pwdnod); @@ -198,22 +202,29 @@ char *path_pwd(void) cp = nv_getval(sh_scoped(HOME)); if(!(cp && *cp=='/' && test_inode(cp,e_dot))) { - /* Get physical PWD (no symlinks) using getcwd(3), fall back to "." */ + /* Get physical PWD (no symlinks) using getcwd(3) */ cp = sh_getcwd(); - if(!cp) - return((char*)e_dot); - tofree++; + if(cp) + tofree++; } /* Store in PWD variable */ - if(sh.subshell) - pwdnod = sh_assignok(pwdnod,1); - nv_putval(pwdnod,cp,NV_RDONLY); + if(cp) + { + if(sh.subshell) + pwdnod = sh_assignok(pwdnod,1); + nv_putval(pwdnod,cp,NV_RDONLY); + } if(tofree) - free(cp); + free((void*)cp); } nv_onattr(pwdnod,NV_EXPORT); + /* Neither obtained the pwd nor can fall back to sane-ish $PWD: fall back to "." */ + if(!cp) + cp = nv_getval(pwdnod); + if(!cp || *cp!='/') + nv_putval(pwdnod,cp=(char*)e_dot,NV_RDONLY); /* Set shell PWD */ - sh.pwd = sh_strdup(pwdnod->nvalue.cp); + sh.pwd = sh_strdup(cp); return((char*)sh.pwd); } diff --git a/src/cmd/ksh93/tests/path.sh b/src/cmd/ksh93/tests/path.sh index 905fd39b4..8da8358e3 100755 --- a/src/cmd/ksh93/tests/path.sh +++ b/src/cmd/ksh93/tests/path.sh @@ -931,5 +931,19 @@ then ( ) 2>/dev/null || err_exit "'source' in POSIX mode does not find ksh function" fi +# ====== +# Crash after unsetting PWD +(unset PWD; (cd /); :) & # the : avoids optimizing out the subshell +wait "$!" 2>/dev/null +((!(e = $?))) || err_exit "shell crashes on 'cd' in subshell exit with unset PWD" \ + "(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"))" +mkdir "$tmp/testdir" +cd "$tmp/testdir" +"$SHELL" -c 'cd /; rmdir "$1"' x "$tmp/testdir" +(unset PWD; exec "$SHELL" -c '(cd /); :') & +wait "$!" 2>/dev/null +((!(e = $?))) || err_exit 'shell crashes on failure obtain the PWD on init' \ + "(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"))" + # ====== exit $((Errors<125?Errors:125))