Re: [PATCH][nitpick-BUG] shebang less scripts and paths starting with -/+

[Stephane Chazelas <stephane@chazelas.org>]

2024-02-04 09:06:27 +0000, Stephane Chazelas:
[...]
> $ echo echo ++ > ++
> $ chmod +x ++
> $ strace -qqfe /exec zsh -c 'PATH=; ++'
> execve("/usr/bin/zsh", ["zsh", "-c", "PATH=; ++"], 0x7fffadb8e870 /* 55 vars */) = 0
> execve("++", ["++"], 0x7ffc57750de8 /* 55 vars */) = -1 ENOEXEC (Exec format error)
> execve("/bin/sh", ["sh", "++"], 0x7ffc57750de8 /* 55 vars */) = 0
> sh: 0: Illegal option -+
> 
> The path of the script was passed as an option to sh rather than
> an operand.
> 
> The patch below changes it to:
> 
> $ strace -qqfe /exec Src/zsh -c 'PATH=; ++'
> execve("Src/zsh", ["Src/zsh", "-c", "PATH=; ++"], 0x7ffd2a923100 /* 55 vars */) = 0
> execve("++", ["++"], 0x7ffd8d292448 /* 55 vars */) = -1 ENOEXEC (Exec format error)
> execve("/bin/sh", ["sh", "-", "++"], 0x7ffd8d292448 /* 55 vars */) = 0
> ++
[...]

Sorry, forgot to add a test case:

diff --git a/Src/exec.c b/Src/exec.c
index 7d8135266..9c8bbb458 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -612,9 +612,22 @@ zexecve(char *pth, char **argv, char **newenvp)
                         }
                     }
 		    if (!isbinary) {
-			argv[-1] = "sh";
+		        char** args = argv;
+			if (argv[0][0] == '-' || argv[0][0] == '+') {
+			    /*
+			     * guard against +foo or -bar script paths being
+			     * taken as options. POSIX says the script path
+			     * must be passed as an *operand*. "--" would also
+			     * make sure the next argument is treated as an
+			     * operand with POSIX compliant sh implementations
+			     * but "-" is more portable (to the Bourne shell in
+			     * particular) and shorter.
+			     */
+			    *--args = "-";
+			}
+			*--args = "sh";
 			winch_unblock();
-			execve("/bin/sh", argv - 1, newenvp);
+			execve("/bin/sh", args, newenvp);
 		    }
 		}
 	    } else
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index bcadc6d56..07a24f9c8 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -2,7 +2,7 @@
 
   storepath=($path)
 
-  mkdir command.tmp command.tmp/dir1 command.tmp/dir2
+  mkdir command.tmp command.tmp/dir{1,2} command.tmp/{+,-}dir
 
   cd command.tmp
 
@@ -21,7 +21,10 @@
   print '#!xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxnyyy' >tstcmd-interp-too-long
   print "#!${sh}\necho should not execute; exit 1" >xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxn
 
-  chmod 755 tstcmd dir1/tstcmd dir2/tstcmd
+  print 'echo no shebang in -dir' > -dir/tstcmd
+  print 'echo no shebang in +dir' > +dir/tstcmd
+
+  chmod 755 tstcmd dir{1,2}/tstcmd ./{-,+}dir/tstcmd
   chmod 755 tstcmd-slashless tstcmd-arg tstcmd-interp-too-long
   chmod 755 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxn
 
@@ -422,3 +425,12 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
  (exit 4); repeat 0 do done
 0:'repeat 0' resets lastval
 
+ -dir/tstcmd
+ +dir/tstcmd
+ PATH=-dir tstcmd
+ PATH=+dir tstcmd
+0:shebang-less scripts are to be run by sh even when their file paths start with - or + (workers/52515)
+>no shebang in -dir
+>no shebang in +dir
+>no shebang in -dir
+>no shebang in +dir