306 lines
9.8 KiB
Diff
306 lines
9.8 KiB
Diff
From aa75399b030dfb0efc39be6d07c627aedbc2d2c1 Mon Sep 17 00:00:00 2001
|
|
From: Johnothan King <johnothanking@protonmail.com>
|
|
Date: Thu, 29 Jan 2026 16:23:45 -0800
|
|
Subject: [PATCH] Fix /dev/fd/1 memory fault and $0 for /dev/fd scripts
|
|
|
|
Changes:
|
|
|
|
- sh_init(): Don't set $0 to the path to ksh. The correct behavior
|
|
is to always use the script name, even when it's a /dev/fd file.
|
|
The behavior of ksh93 should match that of bash and other shells.
|
|
Please read:
|
|
https://github.com/ksh93/ksh/pull/866#issuecomment-2968762167
|
|
https://github.com/ksh93/ksh/issues/874#issuecomment-2977615731
|
|
|
|
- sh_main(): Do not attempt to optimize away sh_open() for the
|
|
stdio fds (0, 1, 2) because that can cause buggy behavior. Ksh's
|
|
handling of /dev/fd/1 and /dev/stdin should not differ. See:
|
|
https://github.com/ksh93/ksh/pull/879#issuecomment-2989729921
|
|
|
|
- exfile(): Don't dup or close fds for /dev/fd scripts with a file
|
|
descriptor > 2. Those should be left open pursuant to the $0 name
|
|
of the script. (This cannot be done safely for fds 1 and 2, so
|
|
the stdio fds are all dup'ed, but not closed.)
|
|
|
|
- sh_main(): If the fd obtained for the script by sh_open() is
|
|
<= 2, that means the shell was opened with one of the stdio fds
|
|
closed, so for correct behavior use sh_iomovefd() to keep the
|
|
stdio fd closed.
|
|
|
|
- sh_iomovefd(): As an optimization, move the fd to a number that's
|
|
either >= 3 or >= 10, depending on what is most appropriate. This
|
|
avoids a possible inefficient scenario of dup'ing an fd from
|
|
0 -> 3 -> 10 (less syscalls is better for performance).
|
|
|
|
- sh_main(): Use O_CLOEXEC in advance for the fd (which is
|
|
guaranteed to get close-on-exec via a later fcntl call; if we
|
|
don't end up calling fcntl for whatever reason O_CLOEXEC will
|
|
help avoid an extra syscall).
|
|
|
|
- basic.sh: Add regression tests for $0 and its associated fd. For
|
|
the time being I've omitted a /dev/fd/1 regression test because I
|
|
can't get one to work correctly with the pty tool.
|
|
|
|
Resolves: https://github.com/ksh93/ksh/issues/874
|
|
Resolves: https://github.com/ksh93/ksh/issues/877
|
|
Resolves: https://github.com/ksh93/ksh/pull/879
|
|
---
|
|
diff --git a/src/cmd/ksh93/include/io.h b/src/cmd/ksh93/include/io.h
|
|
index f3c8ca7..82fad1c 100644
|
|
--- a/src/cmd/ksh93/include/io.h
|
|
+++ b/src/cmd/ksh93/include/io.h
|
|
@@ -66,7 +66,7 @@
|
|
|
|
extern int sh_iocheckfd(int);
|
|
extern void sh_ioinit(void);
|
|
-extern int sh_iomovefd(int);
|
|
+extern int sh_iomovefd(int,int);
|
|
extern int sh_iorenumber(int,int);
|
|
extern void sh_pclose(int[]);
|
|
extern int sh_rpipe(int[]);
|
|
diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
|
|
index ab2ac66..d3f924e 100644
|
|
--- a/src/cmd/ksh93/sh/init.c
|
|
+++ b/src/cmd/ksh93/sh/init.c
|
|
@@ -1432,10 +1432,7 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
|
|
else
|
|
sh_offoption(SH_PRIVILEGED);
|
|
/* shname for $0 in profiles and . scripts */
|
|
- if(sh_isdevfd(argv[1]))
|
|
- sh.shname = sh_strdup(argv[0]);
|
|
- else
|
|
- sh.shname = sh_strdup(sh.st.dolv[0]);
|
|
+ sh.shname = sh_strdup(sh.st.dolv[0]);
|
|
/*
|
|
* return here for shell script execution
|
|
* but not for parenthesis subshells
|
|
diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c
|
|
index 581336a..ea83132 100644
|
|
--- a/src/cmd/ksh93/sh/io.c
|
|
+++ b/src/cmd/ksh93/sh/io.c
|
|
@@ -898,16 +898,16 @@ int sh_chkopen(const char *name)
|
|
}
|
|
|
|
/*
|
|
- * move open file descriptor to a number > 2
|
|
+ * move open file descriptor to a number >= minfd
|
|
*/
|
|
-int sh_iomovefd(int fdold)
|
|
+int sh_iomovefd(int fdold, int minfd)
|
|
{
|
|
int fdnew;
|
|
if(fdold >= sh.lim.open_max)
|
|
sh_iovalidfd(fdold);
|
|
- if(fdold<0 || fdold>2)
|
|
+ if(fdold<0 || fdold>=minfd)
|
|
return fdold;
|
|
- fdnew = sh_iomovefd(dup(fdold));
|
|
+ fdnew = sh_iomovefd(fcntl(fdold,F_DUPFD,minfd),minfd);
|
|
sh.fdstatus[fdnew] = (sh.fdstatus[fdold]&~IOCLEX);
|
|
close(fdold);
|
|
sh.fdstatus[fdold] = IOCLOSE;
|
|
@@ -929,8 +929,8 @@ int sh_pipe(int pv[])
|
|
errormsg(SH_DICT,ERROR_system(1),e_pipe);
|
|
UNREACHABLE();
|
|
}
|
|
- pv[0] = sh_iomovefd(pv[0]);
|
|
- pv[1] = sh_iomovefd(pv[1]);
|
|
+ pv[0] = sh_iomovefd(pv[0],3);
|
|
+ pv[1] = sh_iomovefd(pv[1],3);
|
|
sh.fdstatus[pv[0]] = IONOSEEK|IOREAD;
|
|
sh.fdstatus[pv[1]] = IONOSEEK|IOWRITE;
|
|
sh_subsavefd(pv[0]);
|
|
@@ -954,8 +954,8 @@ int sh_pipe(int pv[])
|
|
errormsg(SH_DICT,ERROR_system(1),e_pipe);
|
|
UNREACHABLE();
|
|
}
|
|
- pv[0] = sh_iomovefd(pv[0]);
|
|
- pv[1] = sh_iomovefd(pv[1]);
|
|
+ pv[0] = sh_iomovefd(pv[0],3);
|
|
+ pv[1] = sh_iomovefd(pv[1],3);
|
|
sh.fdstatus[pv[0]] = IONOSEEK|IOREAD;
|
|
sh.fdstatus[pv[1]] = IONOSEEK|IOWRITE;
|
|
sh_subsavefd(pv[0]);
|
|
@@ -1522,7 +1522,7 @@ int sh_redirect(struct ionod *iop, int flag)
|
|
sh_close(fn);
|
|
}
|
|
if(flag==3)
|
|
- return sh_iomovefd(fd); /* ensure FD > 2 to make $(<file) work with std{in,out,err} closed */
|
|
+ return sh_iomovefd(fd,3); /* ensure FD > 2 to make $(<file) work with std{in,out,err} closed */
|
|
if(fd>=0)
|
|
{
|
|
if(np)
|
|
@@ -1549,7 +1549,7 @@ int sh_redirect(struct ionod *iop, int flag)
|
|
}
|
|
else
|
|
{
|
|
- fd = sh_iorenumber(sh_iomovefd(fd),fn);
|
|
+ fd = sh_iorenumber(sh_iomovefd(fd,3),fn);
|
|
if(fn>2 && fn<10)
|
|
sh.inuse_bits |= (1<<fn);
|
|
}
|
|
diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c
|
|
index 3ec360e..756a9ab 100644
|
|
--- a/src/cmd/ksh93/sh/main.c
|
|
+++ b/src/cmd/ksh93/sh/main.c
|
|
@@ -63,6 +63,9 @@ static void chkmail(char*);
|
|
static struct stat lastmail;
|
|
static time_t mailtime;
|
|
static char beenhere = 0;
|
|
+#if SHOPT_DEVFD
|
|
+#define GOT_DEVFD 2
|
|
+#endif
|
|
|
|
#if _lib_sigvec
|
|
void clearsigmask(int sig)
|
|
@@ -146,7 +149,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
|
|
#endif
|
|
if(!beenhere)
|
|
{
|
|
- beenhere++;
|
|
+ beenhere = 1;
|
|
sh_onstate(SH_PROFILE);
|
|
sh.sigflag[SIGTSTP] |= SH_SIGIGNORE;
|
|
/* decide whether shell is interactive */
|
|
@@ -239,11 +242,10 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
|
|
fdin = 0;
|
|
else
|
|
{
|
|
- char *sp;
|
|
/* open stream should have been passed into shell */
|
|
- if(strmatch(name,e_devfdNN))
|
|
+#if SHOPT_DEVFD
|
|
+ if(strmatch(name,e_devfdNN) && (fdin=(int)strtol(name+8, NULL, 10)) > 2)
|
|
{
|
|
- fdin = (int)strtol(name+8, NULL, 10);
|
|
if(fstat(fdin,&statb)<0)
|
|
{
|
|
errormsg(SH_DICT,ERROR_system(1),e_open,name);
|
|
@@ -252,11 +254,14 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
|
|
name = av[0];
|
|
sh_offoption(SH_VERBOSE);
|
|
sh_offoption(SH_XTRACE);
|
|
+ beenhere = GOT_DEVFD;
|
|
}
|
|
else
|
|
+#endif
|
|
{
|
|
+ char *sp;
|
|
int isdir = 0;
|
|
- if((fdin=sh_open(name,O_RDONLY,0))>=0 &&(fstat(fdin,&statb)<0 || S_ISDIR(statb.st_mode)))
|
|
+ if((fdin=sh_open(name,O_RDONLY|O_cloexec,0))>=0 &&(fstat(fdin,&statb)<0 || S_ISDIR(statb.st_mode)))
|
|
{
|
|
close(fdin);
|
|
isdir = 1;
|
|
@@ -271,7 +276,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
|
|
sp = stkptr(sh.stk,PATH_OFFSET);
|
|
if(sp)
|
|
{
|
|
- if((fdin=sh_open(sp,O_RDONLY,0))>=0)
|
|
+ if((fdin=sh_open(sp,O_RDONLY|O_cloexec,0))>=0)
|
|
sh.st.filename = path_fullname(sp);
|
|
}
|
|
}
|
|
@@ -293,8 +298,14 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
|
|
strcopy(name," \"$@\"");
|
|
goto shell_c;
|
|
}
|
|
- if(fdin==0)
|
|
- fdin = sh_iomovefd(fdin);
|
|
+ /*
|
|
+ * Note: fdin could wind up being zero or some such if the
|
|
+ * shell was opened with stdin/stdout/stderr closed
|
|
+ * (i.e. 0>&-), so we move the fd in such a case to
|
|
+ * keep that file descriptor closed.
|
|
+ */
|
|
+ if(fdin<=2)
|
|
+ fdin = sh_iomovefd(fdin,10);
|
|
}
|
|
sh.readscript = sh.shname;
|
|
}
|
|
@@ -364,15 +375,27 @@ static void exfile(Sfio_t *iop,int fno)
|
|
{
|
|
if(fno > 0)
|
|
{
|
|
- int r;
|
|
- if(fno < 10 && ((r=sh_fcntl(fno,F_DUPFD,10))>=10))
|
|
+#if SHOPT_DEVFD
|
|
+ if(beenhere == GOT_DEVFD)
|
|
+ ; /* leave the inherited /dev/fd as is */
|
|
+ else
|
|
+#endif
|
|
{
|
|
- sh.fdstatus[r] = sh.fdstatus[fno];
|
|
- sh_close(fno);
|
|
- fno = r;
|
|
+ if(fno < 10)
|
|
+ {
|
|
+ int r = sh_fcntl(fno,F_DUPFD,10);
|
|
+ if(r >= 10)
|
|
+ {
|
|
+ sh.fdstatus[r] = sh.fdstatus[fno];
|
|
+ /* leave std* fds open when they're not explicitly closed */
|
|
+ if(fno > 2)
|
|
+ sh_close(fno);
|
|
+ fno = r;
|
|
+ }
|
|
+ }
|
|
+ fcntl(fno,F_SETFD,FD_CLOEXEC);
|
|
+ sh.fdstatus[fno] |= IOCLEX;
|
|
}
|
|
- fcntl(fno,F_SETFD,FD_CLOEXEC);
|
|
- sh.fdstatus[fno] |= IOCLEX;
|
|
iop = sh_iostream(fno);
|
|
}
|
|
else
|
|
@@ -380,6 +403,7 @@ static void exfile(Sfio_t *iop,int fno)
|
|
}
|
|
else
|
|
fno = -1;
|
|
+ beenhere = 1;
|
|
sh.infd = fno;
|
|
if(sh_isstate(SH_INTERACTIVE))
|
|
{
|
|
diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c
|
|
index 06bf66f..36bdc1e 100644
|
|
--- a/src/cmd/ksh93/sh/path.c
|
|
+++ b/src/cmd/ksh93/sh/path.c
|
|
@@ -477,7 +477,7 @@ static int opentype(const char *name, Pathcomp_t *pp, int fun)
|
|
}
|
|
}
|
|
while(fd<0 && nextpp);
|
|
- if(fd>=0 && (fd = sh_iomovefd(fd)) > 0)
|
|
+ if(fd>=0 && (fd = sh_iomovefd(fd,10)) > 0)
|
|
{
|
|
fcntl(fd,F_SETFD,FD_CLOEXEC);
|
|
sh.fdstatus[fd] |= IOCLEX;
|
|
@@ -1292,7 +1292,7 @@ static noreturn void exscript(char *path,char *argv[],char **envp)
|
|
errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec,path);
|
|
UNREACHABLE();
|
|
}
|
|
- sh.infd = sh_iomovefd(sh.infd);
|
|
+ sh.infd = sh_iomovefd(sh.infd,10);
|
|
#if SHOPT_ACCT
|
|
sh_accbegin(path) ; /* reset accounting */
|
|
#endif /* SHOPT_ACCT */
|
|
diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh
|
|
index 5c034e3..38523ed 100755
|
|
--- a/src/cmd/ksh93/tests/basic.sh
|
|
+++ b/src/cmd/ksh93/tests/basic.sh
|
|
@@ -1044,5 +1044,15 @@ unset bindir
|
|
[[ $exp == $got ]] || err_exit 'ksh93 shebang-less scripts are vulnerable to being hijacked for arbitrary code execution' \
|
|
"(exp $(printf %q "$exp"), got $(printf %q "$got"))"
|
|
|
|
+# ======
|
|
+# $0 should be the /dev/fd path for scripts executed from a /dev/fd file
|
|
+# https://github.com/ksh93/ksh/issues/874
|
|
+# https://github.com/ksh93/ksh/pull/879
|
|
+if ((SHOPT_DEVFD))
|
|
+then got=$("$SHELL" <(echo 'echo $0'))
|
|
+ [[ ${got:0:7} != '/dev/fd' ]] && err_exit '$0 is wrong for process substitution scripts' \\
|
|
+ "(expected a /dev/fd file name, got $(printf %q "$got"))"
|
|
+fi
|
|
+
|
|
# ======
|
|
exit $((Errors<125?Errors:125))
|