[PATCH] exec.c: Fix exit status for command -v on non-executable files

* [PATCH] exec.c: Fix exit status for command -v on non-executable files
@ 2022-05-30 17:02 Nicola Lamacchia
  2024-04-05  9:55 ` [v2 PATCH] exec: Check executable bit when searching path Herbert Xu
  0 siblings, 1 reply; 2+ messages in thread
From: Nicola Lamacchia @ 2022-05-30 17:02 UTC (permalink / raw)
  To: dash

Hello,

the following patches `command -v` to make it abide by POSIX
specifications[1] as previously described by orbea in this mailing
list[2]. Behavior comparison at the bottom[5].

Please note that POSIX does not specify the exit status in this
case[1]: "Otherwise, no output shall be written and the exit status
shall reflect that the name was not found." Although it requires the
exit status to be 127 in case of command not found[3].

Also note that in order for a command to be considered "found" when
using the PATH variable, it has to be an executable file[4]: "The list
shall be searched from beginning to end, applying the filename to each
prefix, until an executable file with the specified name and
appropriate execution permissions is found."

Therefore an exit status of 127 seems to be suitable in this case.
Which would leave the exit status 126 to be used when the user tries
to execute a file while not having the right permissions to do so.

---

diff --git a/src/exec.c b/src/exec.c
index 87354d4..facaf6b 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -445,8 +445,8 @@ loop:
                                e = errno;
                        goto loop;
                }
-               e = EACCES;     /* if we fail, this will be the error */
-               if (!S_ISREG(statb.st_mode))
+               e = ENOENT;     /* if we fail, this will be the error */
+               if (!S_ISREG(statb.st_mode) || access(fullname, F_OK|X_OK) != 0)
                        continue;
                if (lpathopt) {         /* this is a %func directory */
                        stalloc(len);
@@ -832,6 +832,9 @@ describe_command(out, command, path, verbose)
                        } while (--j >= 0);
                        p = stackblock();
                }
+               if (access(p, X_OK) != 0) {
+                       goto not_found;
+               }
                if (verbose) {
                        outfmt(
                                out, " is%s %s",
@@ -864,15 +867,18 @@ describe_command(out, command, path, verbose)
                break;

        default:
-               if (verbose) {
-                       outstr(": not found\n", out);
-               }
-               return 127;
+               goto not_found;
        }

 out:
        outc('\n', out);
        return 0;
+
+not_found:
+       if (verbose) {
+               outstr(": not found\n", out);
+       }
+       return 127;
 }

 int
---

Cheers

-- Nicola

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
[2]: https://www.mail-archive.com/dash@vger.kernel.org/msg01968.html
[3]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02
[4]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
[5]:
Old behavior:

$ cd /tmp
$ mkdir -p test1 test2 test3
$ umask 0000
$ touch test1/foo test2/foo test3/foo
$ chmod +x test2/foo
$ PATH=./test1
$ command -v foo
./test1/foo
$ echo $?
0
$ foo
dash: 9: foo: Permission denied
$ echo $?
126
$ test1/foo
dash: 11: test1/foo: Permission denied
$ echo $?
126
$ PATH=./test2
$ command -v foo
./test2/foo
$ echo $?
0
$ foo
$ echo $?
0
$ PATH=./test1:./test2:./test3
$ command -v foo # returns the non-executable file
./test1/foo
$ echo $?
0
$ /bin/chmod +x test1/foo test3/foo
$ command -v foo # returns the cached command
./test1/foo
$ echo $?
0
$ foo # uses the executable file (./test2/foo)
$ echo $?
0
$ /bin/rm test1/foo test2/foo test3/foo
$ /bin/rmdir test1 test2 test3 --ignore-fail-on-non-empty

Patched behavior:

$ cd /tmp
$ mkdir -p test1 test2 test3
$ umask 0000
$ touch test1/foo test2/foo test3/foo
$ chmod +x test2/foo
$ PATH=./test1
$ command -v foo
$ echo $?
127
$ foo
dash: 9: foo: not found
$ echo $?
127
$ test1/foo
dash: 11: test1/foo: Permission denied
$ echo $?
126
$ PATH=./test2
$ command -v foo
./test2/foo
$ echo $?
0
$ foo
$ echo $?
0
$ PATH=./test1:./test2:./test3
$ command -v foo
./test2/foo
$ echo $?
0
$ /bin/chmod +x test1/foo test3/foo
$ command -v foo # returns the cached command
./test2/foo
$ echo $?
0
$ foo
$ echo $?
0
$ /bin/rm test1/foo test2/foo test3/foo
$ /bin/rmdir test1 test2 test3 --ignore-fail-on-non-empty

^ permalink raw reply related	[flat|nested] 2+ messages in thread