From 8bfeb638783dc0a73f5614652928d2f7d161bb7a Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 21 Mar 2024 23:44:24 +0000 Subject: [PATCH] getconf: fix fallback to external (re: c3fec93f, 519bb082, 66e1d446) The previous commit exposed a bug in libcmd's getconf(1) built-in command. First, note that the 'builtin' built-in command can bind a libcmd built-in command to any path: for example, 'builtin /foo/bar/getconf' works. As of 519bb082, path-bound built-ins can be invoked using their canonical path, not just the command name, so invoking a command by its path no longer guarantees that an external command is run. Consequently, if the built-in is bound to the same path that conf.sh detected for the external command, there is an infinite recursion ending in a C stack overflow: b_getconf() in src/lib/libcmd/getconf.c keeps invoking itself via sh_run(). src/lib/libcmd/getconf.c: - b_getconf(): Use 'command -x' to invoke the fallback getconf command to guarantee that no path-bound built-in is run. src/lib/libast/port/astconf.c: - Fix another bug found in lookup() while I was tracing the above one with ASan: an off-by-one error caused a out-of-bounds access just beyond the conf array when looking up certain names. The last element is lo + conf_elements - 1, not lo + conf_elements. - Also fix the confusing way in which these variables are declared. --- NEWS | 5 +++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/tests/path.sh | 15 +++++++++++++++ src/lib/libast/port/astconf.c | 6 +++--- src/lib/libcmd/getconf.c | 9 +++++++-- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 66e343871c20..430a1bf06f3e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,11 @@ This documents significant changes in the 1.0 branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0 Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2024-03-22: + +- Fixed a crash in the getconf built-in that could occur after the 'builtin' + command was used to bind it to the same path as the external getconf command. + 2024-03-20: - Android with Termux is now a supported environment for ksh 93u+m. Testing diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index ecc1684e51b9..190c8ed28e4b 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.9-beta" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2024-03-20" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2024-03-22" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2024 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/tests/path.sh b/src/cmd/ksh93/tests/path.sh index fdacae0bc7b1..b8e44e42c2ca 100755 --- a/src/cmd/ksh93/tests/path.sh +++ b/src/cmd/ksh93/tests/path.sh @@ -318,6 +318,7 @@ if builtin getconf 2> /dev/null; then fi PATH=$path +builtin -d /bin/getconf scr=$tmp/script exp=126 @@ -1022,5 +1023,19 @@ got=${ whence -t whence_t_test 2>&1; } got=${ type -t whence_t_test 2>&1; } [[ $got == "$exp" ]] || err_exit "incorrect 'type -t' output for undefined function (expected '$exp', got '$got')" +# ====== +( + builtin getconf 2>/dev/null || exit 1 + p=$(getconf GETCONF) + [[ $p == /*/getconf ]] || exit 2 + builtin -d getconf + builtin "$p" + PATH=${p%/getconf} + getconf some_nonexistent_config_variable # be sure to trigger fallback to external command + exit 0 +) >/dev/null 2>&1 +(((e = $?) > 1)) && err_exit 'getconf builtin fails when on same path as external getconf' \ + "(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"))" + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/lib/libast/port/astconf.c b/src/lib/libast/port/astconf.c index b9289a103ee5..4780436fc50a 100644 --- a/src/lib/libast/port/astconf.c +++ b/src/lib/libast/port/astconf.c @@ -790,9 +790,9 @@ feature(Feature_t* fp, const char* name, const char* path, const char* value, un static int lookup(Lookup_t* look, const char* name, unsigned int flags) { - Conf_t* mid = (Conf_t*)conf; - Conf_t* lo = mid; - Conf_t* hi = mid + conf_elements; + Conf_t* lo = (Conf_t*)conf; + Conf_t* mid = lo; + Conf_t* hi = lo + conf_elements - 1; int v = 0; int c; char* e; diff --git a/src/lib/libcmd/getconf.c b/src/lib/libcmd/getconf.c index 44bc93dd623c..460fbc04d311 100644 --- a/src/lib/libcmd/getconf.c +++ b/src/lib/libcmd/getconf.c @@ -139,6 +139,7 @@ b_getconf(int argc, char** argv, Shbltin_t* context) int flags; int n; char** oargv; + char** new_argv; static const char empty[] = "-"; cmdinit(argc, argv, context, ERROR_CATALOG, 0); @@ -274,8 +275,12 @@ b_getconf(int argc, char** argv, Shbltin_t* context) /* * Run the external getconf command */ - oargv[0] = native; - if ((n = sh_run(context, argc, oargv)) >= EXIT_NOEXEC) + new_argv = stkalloc(stkstd, (argc + 3) * sizeof(char*)); + new_argv[0] = "command"; + new_argv[1] = "-x"; + new_argv[2] = native; + memcpy(new_argv + 3, oargv + 1, argc * sizeof(char*)); + if ((n = sh_run(context, argc + 2, new_argv)) >= EXIT_NOEXEC) error(ERROR_SYSTEM|2, "%s: exec error [%d]", native, n); return n; }