* Fwd: Bug#874264: dash: 'command -v' mistakenly returns a shell script whose executable is not set
[not found] <150453634811.19057.14500938362227373686.reportbug@homedog.cs.tufts.edu>
@ 2021-11-02 11:12 ` Andrej Shadura
2021-11-10 6:53 ` [PATCH] exec: Check executable bit when searching path Herbert Xu
0 siblings, 1 reply; 2+ messages in thread
From: Andrej Shadura @ 2021-11-02 11:12 UTC (permalink / raw)
To: dash
Hi,
Here’s an old bug from 2017, but it was brought to my attention in some
recent discussion about which "which" is which. There’s also a patch in
one of the follow-ups, but I’m afraid I don’t know enough about that
part of code to judge the consequences of it being applied:
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 <nr@cs.tufts.edu>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Package: dash
Version: 0.5.8-2.4
Severity: normal
Dear Maintainer,
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:
nr@homedog ~/n/s-nail> /bin/sh -c 'command -v clang'
/home/nr/bin/clang
nr@homedog ~/n/s-nail> ls -l /home/nr/bin/clang
-rw-rw-r-- 1 nr nr 1009 Aug 29 2011 /home/nr/bin/clang
nr@homedog ~/n/s-nail> ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Jan 24 2017 /bin/sh -> dash
nr@homedog ~/n/s-nail> ksh -c 'command -v clang'
/usr/bin/clang
nr@homedog ~/n/s-nail> bash -c 'command -v clang'
/usr/bin/clang
nr@homedog ~/n/s-nail> sh -c 'command -v clang'
/home/nr/bin/clang
nr@homedog ~/n/s-nail> dash -c 'command -v clang'
/home/nr/bin/clang
nr@homedog ~/n/s-nail> fish -c 'command -v clang'
/usr/bin/clang
When I run `command -v clang` I expect it to answer /usr/bin/clang.
-- System Information:
Debian Release: 9.1
APT prefers stable
APT policy: (990, 'stable'), (500, 'stable'), (1, 'experimental')
Architecture: i386 (x86_64)
Foreign Architectures: amd64
Kernel: Linux 4.9.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=C, LC_CTYPE=C (charmap=UTF-8) (ignored: LC_ALL set to
en_US.utf8), LANGUAGE=C (charmap=UTF-8) (ignored: LC_ALL set to en_US.utf8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages dash depends on:
ii debianutils 4.8.1.1
ii dpkg 1.18.24
ii libc6 2.24-11+deb9u1
dash recommends no packages.
dash suggests no packages.
-- debconf information:
* dash/sh: true
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH] exec: Check executable bit when searching path
2021-11-02 11:12 ` Fwd: Bug#874264: dash: 'command -v' mistakenly returns a shell script whose executable is not set Andrej Shadura
@ 2021-11-10 6:53 ` Herbert Xu
0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2021-11-10 6:53 UTC (permalink / raw)
To: Andrej Shadura; +Cc: dash, 874264
Andrej Shadura <andrew.shadura@collabora.co.uk> wrote:
>
> Here’s an old bug from 2017, but it was brought to my attention in some
> recent discussion about which "which" is which. There’s also a patch in
> one of the follow-ups, but I’m afraid I don’t know enough about that
> part of code to judge the consequences of it being applied:
>
> 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 <nr@cs.tufts.edu>
> To: Debian Bug Tracking System <submit@bugs.debian.org>
>
> Package: dash
> Version: 0.5.8-2.4
> Severity: normal
>
> Dear Maintainer,
>
>
> 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:
>
> nr@homedog ~/n/s-nail> /bin/sh -c 'command -v clang'
> /home/nr/bin/clang
> nr@homedog ~/n/s-nail> ls -l /home/nr/bin/clang
> -rw-rw-r-- 1 nr nr 1009 Aug 29 2011 /home/nr/bin/clang
> nr@homedog ~/n/s-nail> ls -l /bin/sh
> lrwxrwxrwx 1 root root 4 Jan 24 2017 /bin/sh -> dash
> nr@homedog ~/n/s-nail> ksh -c 'command -v clang'
> /usr/bin/clang
> nr@homedog ~/n/s-nail> bash -c 'command -v clang'
> /usr/bin/clang
> nr@homedog ~/n/s-nail> sh -c 'command -v clang'
> /home/nr/bin/clang
> nr@homedog ~/n/s-nail> dash -c 'command -v clang'
> /home/nr/bin/clang
> nr@homedog ~/n/s-nail> fish -c 'command -v clang'
> /usr/bin/clang
>
> When I run `command -v clang` I expect it to answer /usr/bin/clang.
>
> -- System Information:
> Debian Release: 9.1
> APT prefers stable
> APT policy: (990, 'stable'), (500, 'stable'), (1, 'experimental')
> Architecture: i386 (x86_64)
> Foreign Architectures: amd64
>
> Kernel: Linux 4.9.0-3-amd64 (SMP w/4 CPU cores)
> Locale: LANG=C, LC_CTYPE=C (charmap=UTF-8) (ignored: LC_ALL set to
> en_US.utf8), LANGUAGE=C (charmap=UTF-8) (ignored: LC_ALL set to en_US.utf8)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)
>
> Versions of packages dash depends on:
> ii debianutils 4.8.1.1
> ii dpkg 1.18.24
> ii libc6 2.24-11+deb9u1
>
> dash recommends no packages.
>
> dash suggests no packages.
>
> -- debconf information:
> * dash/sh: true
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.
Reported-by: Norman Ramsey <nr@cs.tufts.edu>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/bltin/test.c b/src/bltin/test.c
index c7fc479..fd8a43b 100644
--- a/src/bltin/test.c
+++ b/src/bltin/test.c
@@ -18,6 +18,7 @@
#include <unistd.h>
#include <stdarg.h>
#include "bltin.h"
+#include "../exec.h"
/* test(1) accepts the following grammar:
oexpr ::= aexpr | aexpr "-o" oexpr ;
@@ -148,11 +149,6 @@ static int isoperand(char **);
static int newerf(const char *, const char *);
static int olderf(const char *, const char *);
static int equalf(const char *, const char *);
-#ifdef HAVE_FACCESSAT
-static int test_file_access(const char *, int);
-#else
-static int test_access(const struct stat64 *, int);
-#endif
#ifdef HAVE_FACCESSAT
# ifdef HAVE_TRADITIONAL_FACCESSAT
@@ -527,7 +523,7 @@ static int has_exec_bit_set(const char *path)
return st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH);
}
-static int test_file_access(const char *path, int mode)
+int test_file_access(const char *path, int mode)
{
if (faccessat_confused_about_superuser() &&
mode == X_OK && geteuid() == 0 && !has_exec_bit_set(path))
@@ -657,7 +653,7 @@ static int test_file_access(const char *path, int mode)
* (euid==uid&&egid==gid), but uses st_mode for '-x' iff running as root.
* i.e. it does strictly conform to 1003.1-2001 (and presumably 1003.2b).
*/
-static int test_access(const struct stat64 *sp, int stmode)
+int test_access(const struct stat64 *sp, int stmode)
{
gid_t *groups;
register int n;
diff --git a/src/exec.c b/src/exec.c
index 87354d4..184717f 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -458,20 +458,14 @@ loop:
stunalloc(fullname);
goto success;
}
-#ifdef notdef
- /* XXX this code stops root executing stuff, and is buggy
- if you need a group from the group list. */
- if (statb.st_uid == geteuid()) {
- if ((statb.st_mode & 0100) == 0)
- goto loop;
- } else if (statb.st_gid == getegid()) {
- if ((statb.st_mode & 010) == 0)
- goto loop;
- } else {
- if ((statb.st_mode & 01) == 0)
- goto loop;
- }
+ if ((statb.st_mode & 0111) != 0111 &&
+#ifdef HAVE_FACCESSAT
+ !test_file_access(fullname, X_OK)
+#else
+ !test_access(&statb, X_OK)
#endif
+ )
+ continue;
TRACE(("searchexec \"%s\" returns \"%s\"\n", name, fullname));
if (!updatetbl) {
entry->cmdtype = CMDNORMAL;
diff --git a/src/exec.h b/src/exec.h
index 423b07e..8707d36 100644
--- a/src/exec.h
+++ b/src/exec.h
@@ -62,6 +62,8 @@ union node;
extern const char *pathopt; /* set by padvance */
+struct stat64;
+
void shellexec(char **, const char *, int)
__attribute__((__noreturn__));
int padvance_magic(const char **path, const char *name, int magic);
@@ -78,6 +80,9 @@ void unsetfunc(const char *);
int typecmd(int, char **);
int commandcmd(int, char **);
+int test_file_access(const char *path, int mode);
+int test_access(const struct stat64 *sp, int stmode);
+
static inline int padvance(const char **path, const char *name)
{
return padvance_magic(path, name, 1);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-10 6:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <150453634811.19057.14500938362227373686.reportbug@homedog.cs.tufts.edu>
2021-11-02 11:12 ` Fwd: Bug#874264: dash: 'command -v' mistakenly returns a shell script whose executable is not set Andrej Shadura
2021-11-10 6:53 ` [PATCH] exec: Check executable bit when searching path Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).