ksh/ksh-1.0.11-devfd-memory-fault.patch
Vincent Mihalkovic 422dba1b91 Fix /dev/fd/1 memory fault and $0 for /dev/fd scripts
Resolves: RHEL-146565
Assisted-by: Cursor <cursoragent@cursor.com>
2026-02-16 10:21:22 +01:00

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))