From 860b3d066f6aaa12dfa0cd2351559e05288cf9b5 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 7 Oct 2024 05:46:31 +0200 Subject: ash: command -v CMD must skip (go to next path) when CMD exists, but is not executable Upstream commit: Date: Fri, 5 Apr 2024 17:55:46 +0800 exec: Check executable bit when searching path Andrej Shadura wrote: ... > https://bugs.debian.org/874264 > -------- Forwarded Message -------- > Subject: dash: 'command -v' mistakenly returns a shell script whose > executable is not set > Date: Mon, 04 Sep 2017 10:45:48 -0400 > From: Norman Ramsey > To: Debian Bug Tracking System ... > I tracked a build bug in s-nail to a problem with dash. Symptom: > building s-nail tries to run /home/nr/bin/clang, a script whose > executable bit is not set. We tracked the problem to the result of > running `command -v clang` with /bin/sh: ... This is inherited from NetBSD. There is even a commented-out block of code that tried to fix this. Anyway, we now have faccessat so we can simply use it. function old new delta test_exec - 125 +125 find_command 911 918 +7 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 1/0 up/down: 132/0) Total: 132 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index 8e029765d..984a71f07 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -491,6 +491,11 @@ struct globals_misc { char **trap_ptr; /* used only by "trap hack" */ /* Rarely referenced stuff */ + + /* Cached supplementary group array (for testing executable'ity of files) */ + int ngroups; + gid_t *group_array; + #if ENABLE_ASH_RANDOM_SUPPORT random_t random_gen; #endif @@ -523,6 +528,8 @@ extern struct globals_misc *BB_GLOBAL_CONST ash_ptr_to_globals_misc; #define may_have_traps (G_misc.may_have_traps ) #define trap (G_misc.trap ) #define trap_ptr (G_misc.trap_ptr ) +#define ngroups (G_misc.ngroups ) +#define group_array (G_misc.group_array) #define random_gen (G_misc.random_gen ) #define backgndpid (G_misc.backgndpid ) #define INIT_G_misc() do { \ @@ -10171,6 +10178,7 @@ static int FAST_FUNC printfcmd(int argc, char **argv) { return printf_main(argc, #endif #if ENABLE_ASH_TEST || BASH_TEST2 static int FAST_FUNC testcmd(int argc, char **argv) { return test_main(argc, argv); } +// TODO: pass &ngroups and &group_array addresses to test_main to use cached supplementary groups #endif #if ENABLE_ASH_SLEEP static int FAST_FUNC sleepcmd(int argc, char **argv) { return sleep_main(argc, argv); } @@ -13782,6 +13790,43 @@ readcmdfile(char *name) /* ============ find_command inplementation */ +static int test_exec(/*const char *fullname,*/ struct stat *statb) +{ + /* + * TODO: use faccessat(AT_FDCWD, fullname, X_OK, AT_EACCESS) + * instead: executability may depend on ACLs, capabilities + * and who knows what else, not just mode bits. + * (faccessat2 syscall was added to Linux in May 14 2020) + */ + mode_t stmode; + uid_t euid; + enum { ANY_IX = S_IXUSR | S_IXGRP | S_IXOTH }; + + /* Do we already know with no extra syscalls? */ + if (!S_ISREG(statb->st_mode)) + return 0; /* not a regular file */ + if ((statb->st_mode & ANY_IX) == 0) + return 0; /* no one can execute */ + if ((statb->st_mode & ANY_IX) == ANY_IX) + return 1; /* anyone can execute */ + + /* Executability depends on our euid/egid/supplementary groups */ + stmode = S_IXOTH; + euid = geteuid(); +//TODO: cache euid? + if (euid == 0) + /* for root user, any X bit is good enough */ + stmode = ANY_IX; + else if (statb->st_uid == euid) + stmode = S_IXUSR; + else if (statb->st_gid == getegid()) + stmode = S_IXGRP; + else if (is_in_supplementary_groups(&ngroups, &group_array, statb->st_gid)) + stmode = S_IXGRP; + + return statb->st_mode & stmode; +} + /* * Resolve a command name. If you change this routine, you may have to * change the shellexec routine as well. @@ -13808,9 +13853,12 @@ find_command(char *name, struct cmdentry *entry, int act, const char *path) if (errno == EINTR) continue; #endif + absfail: entry->cmdtype = CMDUNKNOWN; return; } + if (!test_exec(/*name,*/ &statb)) + goto absfail; } entry->cmdtype = CMDNORMAL; return; @@ -13923,9 +13971,6 @@ find_command(char *name, struct cmdentry *entry, int act, const char *path) e = errno; goto loop; } - e = EACCES; /* if we fail, this will be the error */ - if (!S_ISREG(statb.st_mode)) - continue; if (lpathopt) { /* this is a %func directory */ stalloc(len); /* NB: stalloc will return space pointed by fullname @@ -13938,6 +13983,9 @@ find_command(char *name, struct cmdentry *entry, int act, const char *path) stunalloc(fullname); goto success; } + e = EACCES; /* if we fail, this will be the error */ + if (!test_exec(/*fullname,*/ &statb)) + continue; TRACE(("searchexec \"%s\" returns \"%s\"\n", name, fullname)); if (!updatetbl) { entry->cmdtype = CMDNORMAL; -- cgit v1.2.3-55-g6feb